Skip to content

Commit

Permalink
Remove resolved app directory todos (#43672)
Browse files Browse the repository at this point in the history
All of these todos have been handled right before Next.js conf / shortly after, this PR is cleaning them up.



## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/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`
- [ ] [e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
  • Loading branch information
timneutkens committed Dec 3, 2022
1 parent 9329743 commit 2fcc1e2
Show file tree
Hide file tree
Showing 9 changed files with 1 addition and 36 deletions.
Expand Up @@ -561,7 +561,6 @@ export class FlightClientEntryPlugin {
return shouldInvalidate
}

// TODO-APP: make sure dependsOn is added for layouts/pages
addEntry(
compilation: any,
context: string,
Expand Down
8 changes: 1 addition & 7 deletions packages/next/client/components/app-router.tsx
Expand Up @@ -95,7 +95,6 @@ export async function fetchServerResponse(
}

// Ensure the initialParallelRoutes are not combined because of double-rendering in the browser with Strict Mode.
// TODO-APP: move this back into AppRouter
let initialParallelRoutes: CacheNode['parallelRoutes'] =
typeof window === 'undefined' ? null! : new Map()

Expand Down Expand Up @@ -254,7 +253,6 @@ function Router({
const routerInstance: AppRouterInstance = {
back: () => window.history.back(),
forward: () => window.history.forward(),
// TODO-APP: implement prefetching of flight
prefetch: async (href) => {
// If prefetch has already been triggered, don't trigger it again.
if (prefetched.has(href)) {
Expand All @@ -264,7 +262,6 @@ function Router({
const url = new URL(href, location.origin)
try {
const routerTree = window.history.state?.tree || initialTree
// TODO-APP: handle case where history.state is not the new router history entry
const serverResponse = await fetchServerResponse(
url,
// initialTree is used when history.state.tree is missing because the history state is set in `useEffect` below, it being missing means this is the hydration case.
Expand Down Expand Up @@ -301,8 +298,6 @@ function Router({
React.startTransition(() => {
dispatch({
type: ACTION_REFRESH,

// TODO-APP: revisit if this needs to be passed.
cache: {
status: CacheStates.LAZY_INITIALIZED,
data: null,
Expand Down Expand Up @@ -363,8 +358,7 @@ function Router({
return
}

// 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.
// This case happens when when the history entry was pushed by the `pages` router.
if (!state.__NA) {
window.location.reload()
return
Expand Down
2 changes: 0 additions & 2 deletions packages/next/client/components/layout-router.tsx
Expand Up @@ -209,7 +209,6 @@ export function InnerLayoutRouter({

// If cache node has a data request we have to unwrap response by `use` and update the cache.
if (childNode.data) {
// TODO-APP: error case
/**
* Flight response data
*/
Expand All @@ -229,7 +228,6 @@ export function InnerLayoutRouter({
setTimeout(() => {
// @ts-ignore startTransition exists
React.startTransition(() => {
// TODO-APP: handle redirect
changeByServerResponse(fullTree, flightData, overrideCanonicalUrl)
})
})
Expand Down
1 change: 0 additions & 1 deletion packages/next/client/components/navigation.ts
Expand Up @@ -109,7 +109,6 @@ export {
useServerInsertedHTML,
} from '../../shared/lib/server-inserted-html'

// TODO-APP: Move the other router context over to this one
/**
* Get the router methods. For example router.push('/dashboard')
*/
Expand Down
Expand Up @@ -214,7 +214,6 @@ function getArgumentsForLineNumber(
function guessEditor(): string[] {
// Explicit config always wins
if (process.env.REACT_EDITOR) {
// TODO-APP: fix this type
return shellQuote.parse(process.env.REACT_EDITOR) as any
}

Expand Down
Expand Up @@ -55,7 +55,6 @@ export function useWebsocketPing(
sendMessage(
JSON.stringify({
event: 'ping',
// TODO-APP: fix case for dynamic parameters, this will be resolved wrong currently.
tree,
appDirRoute: true,
})
Expand Down
20 changes: 0 additions & 20 deletions packages/next/client/components/reducer.ts
Expand Up @@ -54,7 +54,6 @@ export function createHrefFromUrl(
/**
* Invalidate cache one level down from the router state.
*/
// TODO-APP: Verify if this needs to be recursive.
function invalidateCacheByRouterState(
newCache: CacheNode,
existingCache: CacheNode,
Expand Down Expand Up @@ -472,12 +471,6 @@ function createOptimisticTree(
result[3] = 'refetch'
}

// TODO-APP: Revisit
// Add url into the tree
// if (isFirstSegment) {
// result[2] = href
// }

return result
}

Expand All @@ -495,10 +488,6 @@ function applyRouterStatePatchToTree(
if (flightSegmentPath.length === 1) {
const tree: FlightRouterState = [...treePatch]

// TODO-APP: revisit
// if (url) {
// tree[2] = url
// }
return tree
}

Expand Down Expand Up @@ -539,11 +528,6 @@ function applyRouterStatePatchToTree(
tree[4] = true
}

// TODO-APP: Revisit
// if (url) {
// tree[2] = url
// }

return tree
}

Expand Down Expand Up @@ -812,7 +796,6 @@ function clientReducer(
canonicalUrl: mutable.canonicalUrlOverride
? mutable.canonicalUrlOverride
: href,
// TODO-APP: verify mpaNavigation not being set is correct here.
pushRef: {
pendingPush,
mpaNavigation: mutable.mpaNavigation,
Expand All @@ -834,7 +817,6 @@ function clientReducer(
canonicalUrl: mutable.canonicalUrlOverride
? mutable.canonicalUrlOverride
: href,
// TODO-APP: verify mpaNavigation not being set is correct here.
pushRef: {
pendingPush,
mpaNavigation: false,
Expand Down Expand Up @@ -877,7 +859,6 @@ function clientReducer(
)

if (hardNavigate) {
// TODO-APP: segments.slice(1) strips '', we can get rid of '' altogether.
// Copy subTreeData for the root node of the cache.
cache.subTreeData = state.cache.subTreeData

Expand Down Expand Up @@ -1333,7 +1314,6 @@ function clientReducer(
const { url, serverResponse } = action
const [flightData, canonicalUrlOverride] = serverResponse

// TODO-APP: Implement prefetch for hard navigation
if (typeof flightData === 'string') {
return state
}
Expand Down
2 changes: 0 additions & 2 deletions packages/next/server/app-render.tsx
Expand Up @@ -1586,7 +1586,6 @@ export async function renderToHTMLOrFlight(
// Flight data that is going to be passed to the browser.
// Currently a single item array but in the future multiple patches might be combined in a single request.
const flightData: FlightData = [
// TODO-APP: change walk to output without ''
(
await walkTreeWithFlightRouterState({
createSegmentPath: (child) => child,
Expand Down Expand Up @@ -1792,7 +1791,6 @@ export async function renderToHTMLOrFlight(
res.statusCode = 404
}

// TODO-APP: show error overlay in development. `element` should probably be wrapped in AppRouter for this case.
const renderStream = await renderToInitialStream({
ReactDOMServer,
element: (
Expand Down
1 change: 0 additions & 1 deletion packages/next/types/index.d.ts
Expand Up @@ -40,7 +40,6 @@ declare module 'react' {
nonce?: string
}

// TODO-APP: check if this is the right type.
function use<T>(promise: Promise<T> | React.Context<T>): T
function cache<T extends Function>(fn: T): T
}
Expand Down

0 comments on commit 2fcc1e2

Please sign in to comment.