parallel routes: remove the per-route default 404 handler #48286

feedthejim · 2023-04-12T09:30:28Z

This PR fixes an issue where throwing a notFound error in a parallel route at the top level at the root level would trigger a notfound boundary at the parallel route level, which meant in practice that you could still see the other slots being rendered below.

This behaviour is undesirable and was caused by the fact that we were inserting a default one at each top-level parallel route. This is not longer needed as we have a global one in app-router.tsx

fixes NEXT-968

ijjk · 2023-04-12T09:59:31Z

Failing test suites

Commit: 2ce8697

pnpm testheadless test/integration/app-document/test/index.test.js

Document and App > Rendering via HTTP > _document > It adds nonces to all scripts and preload links

Expand output

● Document and App › Rendering via HTTP › _document › It adds nonces to all scripts and preload links

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: false

  42 |           if ($(element).attr('nonce') !== nonce) noncesAdded = false
  43 |         })
> 44 |         expect(noncesAdded).toBe(true)
     |                             ^
  45 |       })
  46 |
  47 |       test('It adds crossOrigin to all scripts and preload links', async () => {

  at Object.<anonymous> (integration/app-document/test/rendering.js:44:29)

Read more about building and testing Next.js in contributing.md.

pnpm testheadless test/development/acceptance/ReactRefreshLogBox-builtins.test.ts

ReactRefreshLogBox default > Module not found (missing global CSS)

Expand output

● ReactRefreshLogBox default › Module not found (missing global CSS)

expect(received).toContain(expected) // indexOf

Expected substring: "index page"
Received string:    "<head><meta charset=\"utf-8\"><meta name=\"viewport\" content=\"width=device-width\"><meta name=\"next-head-count\" content=\"2\"><noscript data-n-css=\"\"></noscript><script defer=\"\" nomodule=\"\" src=\"/_next/static/chunks/polyfills.js?ts=1681293717180\"></script><script src=\"/_next/static/chunks/fallback/webpack.js?ts=1681293717180\" defer=\"\"></script><script src=\"/_next/static/chunks/fallback/main.js?ts=1681293717180\" defer=\"\"></script><script src=\"/_next/static/chunks/fallback/pages/_app.js?ts=1681293717180\" defer=\"\"></script><script src=\"/_next/static/chunks/fallback/pages/_error.js?ts=1681293717180\" defer=\"\"></script><noscript id=\"__next_css__DO_NOT_USE__\"></noscript></head><body style=\"\"><div id=\"__next\"></div><script src=\"/_next/static/chunks/fallback/react-refresh.js?ts=1681293717180\"></script><script id=\"__NEXT_DATA__\" type=\"application/json\">{\"props\":{\"pageProps\":{\"statusCode\":500}},\"page\":\"/_error\",\"query\":{},\"buildId\":\"development\",\"isFallback\":false,\"err\":{\"name\":\"Error\",\"source\":\"server\",\"message\":\"Module not found: Can't resolve './non-existent.css'\\n  1 | \\n\\u003e 2 |         import './non-existent.css'\\n    |        ^\\n  3 | \\n  4 |         export default function App({ Component, pageProps }) {\\n  5 |           return \\u003cComponent {...pageProps} /\\u003e\\n\\nhttps://nextjs.org/docs/messages/module-not-found\\n\",\"stack\":\"Error: \\u001b[31m\\u001b[1mModule not found\\u001b[22m\\u001b[39m: Can't resolve '\\u001b[32m./non-existent.css\\u001b[39m'\\n\\u001b[0m \\u001b[90m 1 | \\u001b[39m\\u001b[0m\\n\\u001b[0m\\u001b[31m\\u001b[1m\\u003e\\u001b[22m\\u001b[39m\\u001b[90m 2 | \\u001b[39m        \\u001b[36mimport\\u001b[39m \\u001b[32m'./non-existent.css'\\u001b[39m\\u001b[0m\\n\\u001b[0m \\u001b[90m   | \\u001b[39m       \\u001b[31m\\u001b[1m^\\u001b[22m\\u001b[39m\\u001b[0m\\n\\u001b[0m \\u001b[90m 3 | \\u001b[39m\\u001b[0m\\n\\u001b[0m \\u001b[90m 4 | \\u001b[39m        \\u001b[36mexport\\u001b[39m \\u001b[36mdefault\\u001b[39m \\u001b[36mfunction\\u001b[39m \\u001b[33mApp\\u001b[39m({ \\u001b[33mComponent\\u001b[39m\\u001b[33m,\\u001b[39m pageProps }) {\\u001b[0m\\n\\u001b[0m \\u001b[90m 5 | \\u001b[39m          \\u001b[36mreturn\\u001b[39m \\u001b[33m\\u003c\\u001b[39m\\u001b[33mComponent\\u001b[39m {\\u001b[33m...\\u001b[39mpageProps} \\u001b[33m/\\u001b[39m\\u001b[33m\\u003e\\u001b[39m\\u001b[0m\\n\\nhttps://nextjs.org/docs/messages/module-not-found\\n\\n    at Object.getNotFoundError (/tmp/next-install-cdd367a364956d0c06ba9ea70ecf153d26a2702c51550ecc81b86f71c6edb606/node_modules/.pnpm/file+..+next-repo-6b8b8116eaa93d287e2421489e5f6c6c_bvzagjnauq4g3gdadoluvc6hh4/node_modules/next/dist/build/webpack/plugins/wellknown-errors-plugin/parseNotFoundError.js:112:16)\\n    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)\\n    at async Object.getModuleBuildError (/tmp/next-install-cdd367a364956d0c06ba9ea70ecf153d26a2702c51550ecc81b86f71c6edb606/node_modules/.pnpm/file+..+next-repo-6b8b8116eaa93d287e2421489e5f6c6c_bvzagjnauq4g3gdadoluvc6hh4/node_modules/next/dist/build/webpack/plugins/wellknown-errors-plugin/webpackModuleError.js:97:27)\\n    at async /tmp/next-install-cdd367a364956d0c06ba9ea70ecf153d26a2702c51550ecc81b86f71c6edb606/node_modules/.pnpm/file+..+next-repo-6b8b8116eaa93d287e2421489e5f6c6c_bvzagjnauq4g3gdadoluvc6hh4/node_modules/next/dist/build/webpack/plugins/wellknown-errors-plugin/index.js:15:49\\n    at async Promise.all (index 0)\\n    at async /tmp/next-install-cdd367a364956d0c06ba9ea70ecf153d26a2702c51550ecc81b86f71c6edb606/node_modules/.pnpm/file+..+next-repo-6b8b8116eaa93d287e2421489e5f6c6c_bvzagjnauq4g3gdadoluvc6hh4/node_modules/next/dist/build/webpack/plugins/wellknown-errors-plugin/index.js:13:21\"},\"gip\":true,\"scriptLoader\":[]}</script><div id=\"__next-build-watcher\" style=\"position: fixed; bottom: 10px; right: 20px; width: 0px; height: 0px; z-index: 99999;\"></div><next-route-announcer><p aria-live=\"assertive\" id=\"__next-route-announcer__\" role=\"alert\" style=\"border: 0px; clip: rect(0px, 0px, 0px, 0px); height: 1px; margin: -1px; overflow: hidden; padding: 0px; position: absolute; top: 0px; width: 1px; white-space: nowrap; overflow-wrap: normal;\"></p></next-route-announcer><nextjs-portal></nextjs-portal></body>"

  144 |       expect(
  145 |         await session.evaluate(() => document.documentElement.innerHTML)
> 146 |       ).toContain('index page')
      |         ^
  147 |
  148 |       await cleanup()
  149 |     })

  at Object.<anonymous> (development/acceptance/ReactRefreshLogBox-builtins.test.ts:146:9)

Read more about building and testing Next.js in contributing.md.

pnpm testheadless test/e2e/app-dir/navigation/navigation.test.ts

app dir - navigation > redirect > components > should redirect to external url
app dir - navigation > redirect > next.config.js redirects > should redirect from next.config.js

Expand output

● app dir - navigation › redirect › components › should redirect to external url

thrown: "Exceeded timeout of 240000 ms for a test.
Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

  157 |         })
  158 |
> 159 |         it('should redirect to external url', async () => {
      |         ^
  160 |           const browser = await next.browser('/redirect/external')
  161 |           expect(await browser.waitForElementByCss('h1').text()).toBe(
  162 |             'Example Domain'

  at e2e/app-dir/navigation/navigation.test.ts:159:9
  at e2e/app-dir/navigation/navigation.test.ts:130:7
  at fn (e2e/app-dir/navigation/navigation.test.ts:129:5)
  at lib/e2e-utils.ts:242:5

● app dir - navigation › redirect › next.config.js redirects › should redirect from next.config.js

thrown: "Exceeded timeout of 240000 ms for a test.
Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

  166 |
  167 |       describe('next.config.js redirects', () => {
> 168 |         it('should redirect from next.config.js', async () => {
      |         ^
  169 |           const browser = await next.browser('/redirect/a')
  170 |           expect(await browser.elementByCss('h1').text()).toBe('redirect-dest')
  171 |           expect(await browser.url()).toBe(next.url + '/redirect-dest')

  at e2e/app-dir/navigation/navigation.test.ts:168:9
  at e2e/app-dir/navigation/navigation.test.ts:167:7
  at fn (e2e/app-dir/navigation/navigation.test.ts:129:5)
  at lib/e2e-utils.ts:242:5

Read more about building and testing Next.js in contributing.md.

feedthejim added 2 commits April 12, 2023 10:53

remove default not found boundary for top level parallel routes

4260f7b

add test

dc7f97c

feedthejim requested review from timneutkens, ijjk, shuding and huozhi as code owners April 12, 2023 09:30

ijjk added area: tests created-by: Next.js team PRs by the Next.js team type: next labels Apr 12, 2023

feedthejim enabled auto-merge April 12, 2023 09:32

remove unused

2ce8697

timneutkens approved these changes Apr 12, 2023

View reviewed changes

feedthejim disabled auto-merge April 12, 2023 09:59

feedthejim merged commit 92ddc4a into canary Apr 12, 2023
100 of 102 checks passed

feedthejim deleted the feedthejim/remove-default-canary branch April 12, 2023 11:27

github-actions bot locked as resolved and limited conversation to collaborators May 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

parallel routes: remove the per-route default 404 handler #48286

parallel routes: remove the per-route default 404 handler #48286

feedthejim commented Apr 12, 2023 •

edited

ijjk commented Apr 12, 2023 •

edited

parallel routes: remove the per-route default 404 handler #48286

parallel routes: remove the per-route default 404 handler #48286

Conversation

feedthejim commented Apr 12, 2023 • edited

ijjk commented Apr 12, 2023 • edited

Failing test suites

feedthejim commented Apr 12, 2023 •

edited

ijjk commented Apr 12, 2023 •

edited