Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assign default not-found boundary if custom not-found is not present for root layer only #54185

Merged
merged 4 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions packages/next/src/build/webpack/loaders/next-app-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ const GLOBAL_ERROR_FILE_TYPE = 'global-error'
const PAGE_SEGMENT = 'page$'
const PARALLEL_CHILDREN_SEGMENT = 'children$'

const defaultNotFoundPath = 'next/dist/client/components/not-found-error'

type DirResolver = (pathToResolve: string) => string
type PathResolver = (
pathname: string
Expand Down Expand Up @@ -311,6 +313,14 @@ async function createTreeCodeFromPath(
([, filePath]) => filePath !== undefined
)

// Add default not found error as root not found if not present
const hasNotFound = definedFilePaths.some(
([type]) => type === 'not-found'
)
if (isRootLayer && !hasNotFound) {
definedFilePaths.push(['not-found', defaultNotFoundPath])
}

if (!rootLayout) {
const layoutPath = definedFilePaths.find(
([type]) => type === 'layout'
Expand All @@ -321,17 +331,6 @@ async function createTreeCodeFromPath(
globalError = await resolver(
`${path.dirname(layoutPath)}/${GLOBAL_ERROR_FILE_TYPE}`
)

const hasNotFound = definedFilePaths.some(
([type]) => type === 'not-found'
)
// Add default not found error as root not found if not present
if (!hasNotFound) {
const notFoundPath = 'next/dist/client/components/not-found-error'
if (notFoundPath) {
definedFilePaths.push(['not-found', notFoundPath])
}
}
}
}

Expand All @@ -350,7 +349,7 @@ async function createTreeCodeFromPath(
if (isNotFoundRoute && normalizedParallelKey === 'children') {
const notFoundPath =
definedFilePaths.find(([type]) => type === 'not-found')?.[1] ??
'next/dist/client/components/not-found-error'
defaultNotFoundPath
subtreeCode = `{
children: ['__PAGE__', {}, {
page: [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { notFound } from 'next/navigation'

// avoid static generation to fill the dynamic params
export const dynamic = 'force-dynamic'

export default function Page({ params: { id } }) {
if (id === '404') {
notFound()
}

return <p id="page">{`dynamic-layout-without-not-found [id]`}</p>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export default function Layout({ children }) {
return (
<div>
<h1>Dynamic with Layout</h1>
{children}
</div>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <div>dynamic-with-layout</div>
}
8 changes: 5 additions & 3 deletions test/e2e/app-dir/not-found/basic/app/dynamic/[id]/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ import { notFound } from 'next/navigation'
// avoid static generation to fill the dynamic params
export const dynamic = 'force-dynamic'

export default function Page() {
notFound()
export default function Page({ params: { id } }) {
if (id === '404') {
notFound()
}

return <>{`dynamic [id]`}</>
return <p id="page">{`dynamic [id]`}</p>
}
2 changes: 1 addition & 1 deletion test/e2e/app-dir/not-found/basic/app/not-found.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
export default function NotFound() {
return (
<>
<h1>This Is The Not Found Page</h1>
<h1>Root Not Found</h1>

<div id="timestamp">{Date.now()}</div>
</>
Expand Down
46 changes: 36 additions & 10 deletions test/e2e/app-dir/not-found/basic/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ createNextDescribe(
it('should use root not-found content for 404 html', async () => {
// static /404 page will use /_not-found content
const page404 = await next.readFile('.next/server/pages/404.html')
expect(page404).toContain('This Is The Not Found Page')
expect(page404).toContain('Root Not Found')
})
}

const runTests = ({ isEdge }: { isEdge: boolean }) => {
it('should use the not-found page for non-matching routes', async () => {
const browser = await next.browser('/random-content')
expect(await browser.elementByCss('h1').text()).toContain(
'This Is The Not Found Page'
'Root Not Found'
)
// should contain root layout content
expect(await browser.elementByCss('#layout-nav').text()).toBe('Navbar')
Expand All @@ -48,11 +48,40 @@ createNextDescribe(
const browserDynamic = await next.browser('/dynamic')
expect(await browserDynamic.elementByCss('main').text()).toBe('dynamic')

// `/dynamic/[id]` calling notFound() will match the same level not-found boundary
const browserDynamicId = await next.browser('/dynamic/123')
expect(await browserDynamicId.elementByCss('#not-found').text()).toBe(
// `/dynamic/404` calling notFound() will match the same level not-found boundary
const browserDynamic404 = await next.browser('/dynamic/404')
expect(await browserDynamic404.elementByCss('#not-found').text()).toBe(
'dynamic/[id] not found'
)

const browserDynamicId = await next.browser('/dynamic/123')
expect(await browserDynamicId.elementByCss('#page').text()).toBe(
'dynamic [id]'
)
})

it('should escalate notFound to parent layout if no not-found boundary present in current layer', async () => {
const browserDynamic = await next.browser(
'/dynamic-layout-without-not-found'
)
expect(await browserDynamic.elementByCss('h1').text()).toBe(
'Dynamic with Layout'
)

// no not-found boundary in /dynamic-layout-without-not-found, escalate to parent layout to render root not-found
const browserDynamicId = await next.browser(
'/dynamic-layout-without-not-found/404'
)
expect(await browserDynamicId.elementByCss('h1').text()).toBe(
'Root Not Found'
)

const browserDynamic404 = await next.browser(
'/dynamic-layout-without-not-found/123'
)
expect(await browserDynamic404.elementByCss('#page').text()).toBe(
'dynamic-layout-without-not-found [id]'
)
})

if (isNextDev) {
Expand All @@ -74,10 +103,7 @@ createNextDescribe(
const browser = await next.browser('/')
await check(() => browser.elementByCss('h1').text(), 'My page')
await next.renameFile('./app/page.js', './app/foo.js')
await check(
() => browser.elementByCss('h1').text(),
'This Is The Not Found Page'
)
await check(() => browser.elementByCss('h1').text(), 'Root Not Found')
await next.renameFile('./app/foo.js', './app/page.js')
await check(() => browser.elementByCss('h1').text(), 'My page')
})
Expand All @@ -86,7 +112,7 @@ createNextDescribe(
if (!isNextDev && !isEdge) {
it('should create the 404 mapping and copy the file to pages', async () => {
const html = await next.readFile('.next/server/pages/404.html')
expect(html).toContain('This Is The Not Found Page')
expect(html).toContain('Root Not Found')
expect(
await next.readFile('.next/server/pages-manifest.json')
).toContain('"pages/404.html"')
Expand Down