Skip to content

Commit

Permalink
Rename todos (#38560)
Browse files Browse the repository at this point in the history
Renames todos to split them / make it easier to find what has to be done for the new router.


## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm lint`
- [ ] The examples guidelines are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing.md#adding-examples)
  • Loading branch information
timneutkens committed Jul 12, 2022
1 parent 62eb16b commit 81b554f
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 76 deletions.
2 changes: 1 addition & 1 deletion packages/next/build/entries.ts
Expand Up @@ -510,7 +510,7 @@ export function finalizeEntrypoint({
name !== CLIENT_STATIC_FILES_RUNTIME_AMP &&
name !== CLIENT_STATIC_FILES_RUNTIME_REACT_REFRESH
) {
// TODO: this is a temporary fix. @shuding is going to change the handling of server components
// TODO-APP: this is a temporary fix. @shuding is going to change the handling of server components
if (appDir && entry.import.includes('flight')) {
return {
dependOn: CLIENT_STATIC_FILES_RUNTIME_MAIN_ROOT,
Expand Down
6 changes: 3 additions & 3 deletions packages/next/client/app-next-dev.js
@@ -1,8 +1,8 @@
import { hydrate, version } from './app-index'

// TODO: implement FOUC guard
// TODO-APP: implement FOUC guard

// TODO: hydration warning
// TODO-APP: hydration warning

window.next = {
version,
Expand All @@ -11,4 +11,4 @@ window.next = {

hydrate()

// TODO: build indicator
// TODO-APP: build indicator
12 changes: 6 additions & 6 deletions packages/next/client/components/app-router.client.tsx
Expand Up @@ -49,7 +49,7 @@ function ErrorOverlay({
}
}

// TODO: move this back into AppRouter
// TODO-APP: move this back into AppRouter
let initialParallelRoutes: CacheNode['parallelRoutes'] =
typeof window === 'undefined' ? null! : new Map()

Expand Down Expand Up @@ -136,7 +136,7 @@ export default function AppRouter({
}

const routerInstance: AppRouterInstance = {
// TODO: implement prefetching of loading / flight
// TODO-APP: implement prefetching of loading / flight
prefetch: (_href) => Promise.resolve(),
replace: (href) => {
// @ts-ignore startTransition exists
Expand Down Expand Up @@ -168,7 +168,7 @@ export default function AppRouter({
dispatch({
type: 'reload',
payload: {
// TODO: revisit if this needs to be passed.
// TODO-APP: revisit if this needs to be passed.
url: new URL(window.location.href),
cache: {
data: null,
Expand Down Expand Up @@ -211,19 +211,19 @@ export default function AppRouter({

const onPopState = React.useCallback(({ state }: PopStateEvent) => {
if (!state) {
// TODO: this case only happens when pushState/replaceState was called outside of Next.js. It should probably reload the page in this case.
// TODO-APP: this case only happens when pushState/replaceState was called outside of Next.js. It should probably reload the page in this case.
return
}

// TODO: this case happens when pushState/replaceState was called outside of Next.js or when the history entry was pushed by the old router.
// TODO-APP: this case happens when pushState/replaceState was called outside of Next.js or when the history entry was pushed by the old router.
// It reloads the page in this case but we might have to revisit this as the old router ignores it.
if (!state.__NA) {
window.location.reload()
return
}

// @ts-ignore useTransition exists
// TODO: Ideally the back button should not use startTransition as it should apply the updates synchronously
// TODO-APP: Ideally the back button should not use startTransition as it should apply the updates synchronously
// Without startTransition works if the cache is there for this path
React.startTransition(() => {
dispatch({
Expand Down
6 changes: 3 additions & 3 deletions packages/next/client/components/hooks-client.ts
Expand Up @@ -21,12 +21,12 @@ export function useSearchParam(key: string) {
return params[key]
}

// TODO: Move the other router context over to this one
// TODO-APP: Move the other router context over to this one
export function useRouter() {
return useContext(AppRouterContext)
}

// TODO: getting all params when client-side navigating is non-trivial as it does not have route matchers so this might have to be a server context instead.
// TODO-APP: getting all params when client-side navigating is non-trivial as it does not have route matchers so this might have to be a server context instead.
// export function useParams() {
// return useContext(ParamsContext)
// }
Expand All @@ -35,7 +35,7 @@ export function usePathname() {
return useContext(PathnameContext)
}

// TODO: define what should be provided through context.
// TODO-APP: define what should be provided through context.
// export function useLayoutSegments() {
// return useContext(LayoutSegmentsContext)
// }
Expand Down
14 changes: 7 additions & 7 deletions packages/next/client/components/hot-reloader.client.tsx
Expand Up @@ -3,7 +3,7 @@ import {
useContext,
useEffect,
useRef,
// @ts-expect-error TODO: startTransition exists
// @ts-expect-error TODO-APP: startTransition exists
startTransition,
} from 'react'
import { FullAppTreeContext } from '../../shared/lib/app-router-context'
Expand Down Expand Up @@ -31,7 +31,7 @@ function getSocketProtocol(assetPrefix: string): string {

// const TIMEOUT = 5000

// TODO: add actual type
// TODO-APP: add actual type
type PongEvent = any

let mostRecentCompilationHash: any = null
Expand Down Expand Up @@ -305,7 +305,7 @@ function processMessage(
}
return
}
// TODO: make server component change more granular
// TODO-APP: make server component change more granular
case 'serverComponentChanges': {
sendMessage(
JSON.stringify({
Expand Down Expand Up @@ -375,7 +375,7 @@ function processMessage(
// Page exists now, reload
location.reload()
} else {
// TODO: fix this
// TODO-APP: fix this
// Page doesn't exist
// if (
// self.__NEXT_DATA__.page === Router.pathname &&
Expand Down Expand Up @@ -442,12 +442,12 @@ export default function HotReload({ assetPrefix }: { assetPrefix: string }) {
}, [assetPrefix])
useEffect(() => {
// Taken from on-demand-entries-client.js
// TODO: check 404 case
// TODO-APP: check 404 case
const interval = setInterval(() => {
sendMessage(
JSON.stringify({
event: 'ping',
// TODO: fix case for dynamic parameters, this will be resolved wrong currently.
// TODO-APP: fix case for dynamic parameters, this will be resolved wrong currently.
tree,
appDirRoute: true,
})
Expand All @@ -459,7 +459,7 @@ export default function HotReload({ assetPrefix }: { assetPrefix: string }) {
const handler = (event: MessageEvent<PongEvent>) => {
if (
event.data.indexOf('action') === -1 &&
// TODO: clean this up for consistency
// TODO-APP: clean this up for consistency
event.data.indexOf('pong') === -1
) {
return
Expand Down
12 changes: 6 additions & 6 deletions packages/next/client/components/layout-router.client.tsx
Expand Up @@ -123,7 +123,7 @@ export function InnerLayoutRouter({
return treeToRecreate
}

// TODO: remove ''
// TODO-APP: remove ''
const refetchTree = walkAddRefetch(['', ...segmentPath], fullTree)

const data = fetchServerResponse(new URL(url, location.origin), refetchTree)
Expand All @@ -148,7 +148,7 @@ export function InnerLayoutRouter({
}

if (childNode.data) {
// TODO: error case
// TODO-APP: error case
const flightData = childNode.data.readRoot()

// Handle case when navigating to page in `pages` from `app`
Expand All @@ -165,7 +165,7 @@ export function InnerLayoutRouter({
if (pathMatches(flightDataPath, segmentPath)) {
childNode.data = null
// Last item is the subtreeData
// TODO: routerTreePatch needs to be applied to the tree, handle it in render?
// TODO-APP: routerTreePatch needs to be applied to the tree, handle it in render?
const [, /* routerTreePatch */ subTreeData] = flightDataPath.slice(-2)
childNode.subTreeData = subTreeData
childNode.parallelRoutes = new Map()
Expand All @@ -182,7 +182,7 @@ export function InnerLayoutRouter({
setTimeout(() => {
// @ts-ignore startTransition exists
React.startTransition(() => {
// TODO: handle redirect
// TODO-APP: handle redirect
changeByServerResponse(fullTree, flightData)
})
})
Expand All @@ -191,7 +191,7 @@ export function InnerLayoutRouter({
}
}

// TODO: double check users can't return null in a component that will kick in here
// TODO-APP: double check users can't return null in a component that will kick in here
if (!childNode.subTreeData) {
throw createInfinitePromise()
}
Expand All @@ -201,7 +201,7 @@ export function InnerLayoutRouter({
value={{
tree: tree[1][parallelRouterKey],
childNodes: childNode.parallelRoutes,
// TODO: overriding of url for parallel routes
// TODO-APP: overriding of url for parallel routes
url: url,
}}
>
Expand Down
26 changes: 13 additions & 13 deletions packages/next/client/components/reducer.ts
Expand Up @@ -12,7 +12,7 @@ const fillCacheWithNewSubTreeData = (
existingCache: CacheNode,
flightDataPath: FlightDataPath
) => {
// TODO: handle case of / (root of the tree) refetch
// TODO-APP: handle case of / (root of the tree) refetch
const isLastEntry = flightDataPath.length <= 4
const [parallelRouteKey, segment] = flightDataPath

Expand Down Expand Up @@ -224,7 +224,7 @@ const walkTreeWithFlightDataPath = (
const [currentSegment, parallelRouteKey] = flightSegmentPath

// Tree path returned from the server should always match up with the current tree in the browser
// TODO: verify
// TODO-APP: verify
if (!matchSegment(currentSegment, segment)) {
throw new Error('SEGMENT MISMATCH')
}
Expand Down Expand Up @@ -315,7 +315,7 @@ export function reducer(
const href = url.pathname + url.search + url.hash

const segments = pathname.split('/')
// TODO: figure out something better for index pages
// TODO-APP: figure out something better for index pages
segments.push('')

// In case of soft push data fetching happens in layout-router if a segment is missing
Expand Down Expand Up @@ -352,7 +352,7 @@ export function reducer(
}
}

// TODO: flag on the tree of which part of the tree for if there is a loading boundary
// TODO-APP: flag on the tree of which part of the tree for if there is a loading boundary
const isOptimistic = false

if (isOptimistic) {
Expand All @@ -367,7 +367,7 @@ export function reducer(
)

// Fill in the cache with blank that holds the `data` field.
// TODO: segments.slice(1) strips '', we can get rid of '' altogether.
// TODO-APP: segments.slice(1) strips '', we can get rid of '' altogether.
const res = fillCacheWithDataProperty(
cache,
state.cache,
Expand Down Expand Up @@ -406,13 +406,13 @@ export function reducer(

cache.data = null

// TODO: ensure flightDataPath does not have "" as first item
// TODO-APP: ensure flightDataPath does not have "" as first item
const flightDataPath = flightData[0]

const [treePatch] = flightDataPath.slice(-2)
const treePath = flightDataPath.slice(0, -3)
const newTree = walkTreeWithFlightDataPath(
// TODO: remove ''
// TODO-APP: remove ''
['', ...treePath],
state.tree,
treePatch
Expand All @@ -438,7 +438,7 @@ export function reducer(
if (action.type === 'server-patch') {
const { flightData, previousTree, cache } = action.payload
if (JSON.stringify(previousTree) !== JSON.stringify(state.tree)) {
// TODO: Handle tree mismatch
// TODO-APP: Handle tree mismatch
console.log('TREE MISMATCH')
return {
canonicalUrl: state.canonicalUrl,
Expand All @@ -458,15 +458,15 @@ export function reducer(
}
}

// TODO: flightData could hold multiple paths
// TODO-APP: flightData could hold multiple paths
const flightDataPath = flightData[0]

// Slices off the last segment (which is at -3) as it doesn't exist in the tree yet
const treePath = flightDataPath.slice(0, -3)
const [treePatch] = flightDataPath.slice(-2)

const newTree = walkTreeWithFlightDataPath(
// TODO: remove ''
// TODO-APP: remove ''
['', ...treePath],
state.tree,
treePatch
Expand Down Expand Up @@ -526,18 +526,18 @@ export function reducer(

cache.data = null

// TODO: ensure flightDataPath does not have "" as first item
// TODO-APP: ensure flightDataPath does not have "" as first item
const flightDataPath = flightData[0]

if (flightDataPath.length !== 2) {
// TODO: handle this case better
// TODO-APP: handle this case better
console.log('RELOAD FAILED')
return state
}

const [treePatch, subTreeData] = flightDataPath.slice(-2)
const newTree = walkTreeWithFlightDataPath(
// TODO: remove ''
// TODO-APP: remove ''
[''],
state.tree,
treePatch
Expand Down
4 changes: 2 additions & 2 deletions packages/next/client/link.tsx
Expand Up @@ -46,7 +46,7 @@ type InternalLinkProps = {
onClick?: (e: any) => void
}

// TODO: Include the full set of Anchor props
// TODO-APP: Include the full set of Anchor props
// adding this to the publicly exported type currently breaks existing apps
export type LinkProps = InternalLinkProps
type LinkPropsRequired = RequiredKeys<LinkProps>
Expand Down Expand Up @@ -292,7 +292,7 @@ const Link = React.forwardRef<HTMLAnchorElement, LinkPropsReal>(
: []
let router = React.useContext(RouterContext)

// TODO: type error. Remove `as any`
// TODO-APP: type error. Remove `as any`
const appRouter = React.useContext(AppRouterContext) as any
if (appRouter) {
router = appRouter
Expand Down
17 changes: 0 additions & 17 deletions packages/next/lib/app-layout.tsx

This file was deleted.

0 comments on commit 81b554f

Please sign in to comment.