Skip to content

Commit

Permalink
Fix middleware notFound: true handling (#46759)
Browse files Browse the repository at this point in the history
This ensures we properly handle rendering the `404` page when `notFound:
true` is returned and middleware is present.

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see
[`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

Fixes: #38239
x-ref: #43772

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
  • Loading branch information
ijjk and kodiakhq[bot] committed Mar 4, 2023
1 parent 6357de0 commit 3e9a99c
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 3 deletions.
15 changes: 12 additions & 3 deletions packages/next/src/shared/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ function fetchNextData({
return { dataHref, response, text, json: {}, cacheKey }
}

if (!hasMiddleware && response.status === 404) {
if (response.status === 404) {
if (tryToParseAsJSON(text)?.notFound) {
return {
dataHref,
Expand Down Expand Up @@ -1647,6 +1647,7 @@ export default class Router implements BaseRouter {
routeProps: { shallow: false },
locale: nextState.locale,
isPreview: nextState.isPreview,
isNotFound: true,
})

if ('type' in routeInfo) {
Expand Down Expand Up @@ -1913,6 +1914,7 @@ export default class Router implements BaseRouter {
unstable_skipClientCache,
isQueryUpdating,
isMiddlewareRewrite,
isNotFound,
}: {
route: string
pathname: string
Expand All @@ -1926,6 +1928,7 @@ export default class Router implements BaseRouter {
unstable_skipClientCache?: boolean
isQueryUpdating?: boolean
isMiddlewareRewrite?: boolean
isNotFound?: boolean
}) {
/**
* This `route` binding can change if there's a rewrite
Expand Down Expand Up @@ -1959,7 +1962,7 @@ export default class Router implements BaseRouter {
dataHref: this.pageLoader.getDataHref({
href: formatWithValidation({ pathname, query }),
skipInterpolation: true,
asPath: resolvedAs,
asPath: isNotFound ? '/404' : resolvedAs,
locale,
}),
hasMiddleware: true,
Expand All @@ -1981,7 +1984,7 @@ export default class Router implements BaseRouter {
? null
: await withMiddlewareEffects({
fetchData: () => fetchNextData(fetchNextDataParams),
asPath: resolvedAs,
asPath: isNotFound ? '/404' : resolvedAs,
locale: locale,
router: this,
}).catch((err) => {
Expand All @@ -1995,6 +1998,12 @@ export default class Router implements BaseRouter {
throw err
})

// when rendering error routes we don't apply middleware
// effects
if (data && (pathname === '/_error' || pathname === '/404')) {
data.effect = undefined
}

if (isQueryUpdating) {
if (!data) {
data = { json: self.__NEXT_DATA__.props }
Expand Down
11 changes: 11 additions & 0 deletions test/e2e/middleware-general/app/pages/ssg/[slug].js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ export default function Page(props) {
router.isReady ? router.asPath : router.href
)

if (!props.params) {
console.error('props', props)
throw new Error('missing props!!!')
}

useEffect(() => {
if (router.isReady) {
setAsPath(router.asPath)
Expand All @@ -26,6 +31,12 @@ export default function Page(props) {
}

export function getStaticProps({ params }) {
if (params.slug.includes('not-found')) {
return {
notFound: true,
}
}

return {
props: {
now: Date.now(),
Expand Down
12 changes: 12 additions & 0 deletions test/e2e/middleware-general/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,18 @@ describe('Middleware Runtime', () => {
}

function runTests({ i18n }: { i18n?: boolean }) {
it('should work with notFound: true correctly', async () => {
const browser = await next.browser('/ssr-page')
await browser.eval('window.beforeNav = 1')
await browser.eval('window.next.router.push("/ssg/not-found-1")')

await check(
() => browser.eval('document.documentElement.innerHTML'),
/This page could not be found/
)
expect(await browser.eval('window.beforeNav')).toBe(1)
})

it('should be able to rewrite on _next/static/chunks/pages/ 404', async () => {
const res = await fetchViaHTTP(
next.url,
Expand Down

0 comments on commit 3e9a99c

Please sign in to comment.