mirror of
https://github.com/hanzoai/next.js.git
synced 2026-09-24 17:00:51 +00:00
danandGitHub
0950e8ae05
Not sure if there's any downside to this. Currently, there are situations when a browser requests something in a particular format: - Favicons - Manifest icons - `<img>` with broken `src` - Wrong `<link>`s - `new Worker()` These requests have a `Sec-Fetch-Dest` header which can definitely exclude things that can't be HTML. My proposal is to not return the HTML for them (even in dev) so that this doesn't waste time in case rendering the root layout is slow or expensive. Less work for the server, whether dev or prod. I believe this was happening in dev, and in prod if rendering `not-found` requires something dynamic (e.g. in the root layout). And for static self-hosted, I believe it would respond with static HTML. After this change, we don't render the HTML. ## Before Image in manifest hits the real `/not-found` route and runs application code: <img width="1055" height="639" alt="Screenshot 2026-07-19 at 00 23 01" src="https://github.com/user-attachments/assets/869f85f8-a009-4266-a345-74e3342adb41" /> Notice 404 at the end with 2 seconds in application code. ## After Image in manifest is still 404 but doesn't hit any app code. <img width="1033" height="696" alt="Screenshot 2026-07-19 at 00 21 17" src="https://github.com/user-attachments/assets/3a1817b4-f050-45c0-93ca-0507cf9de803" /> It's maybe a bit weird it doesn't show in the terminal log. In that sense it's like `/_next/static/` because it's handled at an earlier layer. Maybe we could still log it but it would have to happen from a different place, and also breaks consistency, so I opted with keeping it hidden. --- I can't see any coherent arguments for why this should reach the app render so this should be OK?
109 lines
4.0 KiB
TypeScript
109 lines
4.0 KiB
TypeScript
import { isNextDeploy, nextTestSetup } from 'e2e-utils'
|
|
import { retry } from 'next-test-utils'
|
|
|
|
describe('not-found-non-document', () => {
|
|
const { next } = nextTestSetup({
|
|
files: __dirname,
|
|
})
|
|
|
|
it('returns a plain text 404 for subresource requests to unknown paths', async () => {
|
|
const outputIndex = next.cliOutput.length
|
|
const res = await next.fetch('/web-app-manifest-192x192.png', {
|
|
headers: {
|
|
accept: 'image/avif,image/webp,image/png,image/svg+xml,image/*;q=0.8',
|
|
'sec-fetch-dest': 'image',
|
|
'sec-fetch-mode': 'no-cors',
|
|
'sec-fetch-site': 'same-origin',
|
|
},
|
|
})
|
|
expect(res.status).toBe(404)
|
|
if (isNextDeploy) {
|
|
// The build emits a prerendered not-found page, and the deployed
|
|
// routing layer serves it without invoking Next.js.
|
|
// TODO: It might be good to align the CDN behavior so subresource
|
|
// requests to unknown paths get the plain text 404 when deployed too.
|
|
expect(res.headers.get('content-type')).toContain('text/html')
|
|
} else {
|
|
expect(res.headers.get('content-type')).toContain('text/plain')
|
|
expect(res.headers.get('cache-control')).toBe(
|
|
'private, no-cache, no-store, max-age=0, must-revalidate'
|
|
)
|
|
expect(await res.text()).toBe('Not Found')
|
|
expect(next.cliOutput.slice(outputIndex)).not.toContain(
|
|
'__not-found-component-rendered__'
|
|
)
|
|
}
|
|
})
|
|
|
|
it('returns a plain text 404 for font and manifest requests to unknown paths', async () => {
|
|
for (const dest of ['font', 'manifest']) {
|
|
const res = await next.fetch(`/missing-${dest}`, {
|
|
headers: {
|
|
'sec-fetch-dest': dest,
|
|
'sec-fetch-mode': 'no-cors',
|
|
'sec-fetch-site': 'same-origin',
|
|
},
|
|
})
|
|
expect(res.status).toBe(404)
|
|
if (isNextDeploy) {
|
|
// The build emits a prerendered not-found page, and the deployed
|
|
// routing layer serves it without invoking Next.js.
|
|
// TODO: It might be good to align the CDN behavior so subresource
|
|
// requests to unknown paths get the plain text 404 when deployed too.
|
|
expect(res.headers.get('content-type')).toContain('text/html')
|
|
} else {
|
|
expect(res.headers.get('content-type')).toContain('text/plain')
|
|
expect(res.headers.get('cache-control')).toBe(
|
|
'private, no-cache, no-store, max-age=0, must-revalidate'
|
|
)
|
|
expect(await res.text()).toBe('Not Found')
|
|
}
|
|
}
|
|
})
|
|
|
|
it('renders the not-found page for document requests to unknown paths', async () => {
|
|
const res = await next.fetch('/does-not-exist', {
|
|
headers: {
|
|
accept:
|
|
'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8',
|
|
'sec-fetch-dest': 'document',
|
|
'sec-fetch-mode': 'navigate',
|
|
'sec-fetch-site': 'none',
|
|
},
|
|
})
|
|
expect(res.status).toBe(404)
|
|
expect(res.headers.get('content-type')).toContain('text/html')
|
|
expect(await res.text()).toContain('custom not found page')
|
|
})
|
|
|
|
it('renders the not-found page for requests without sec-fetch-dest', async () => {
|
|
const res = await next.fetch('/does-not-exist')
|
|
expect(res.status).toBe(404)
|
|
expect(res.headers.get('content-type')).toContain('text/html')
|
|
expect(await res.text()).toContain('custom not found page')
|
|
})
|
|
|
|
it('renders the not-found page for fetch requests to unknown paths', async () => {
|
|
const res = await next.fetch('/does-not-exist', {
|
|
headers: {
|
|
'sec-fetch-dest': 'empty',
|
|
'sec-fetch-mode': 'cors',
|
|
'sec-fetch-site': 'same-origin',
|
|
},
|
|
})
|
|
expect(res.status).toBe(404)
|
|
expect(res.headers.get('content-type')).toContain('text/html')
|
|
expect(await res.text()).toContain('custom not found page')
|
|
})
|
|
|
|
it('renders the not-found page on client navigation to unknown paths', async () => {
|
|
const browser = await next.browser('/')
|
|
await browser.elementByCss('#link-to-missing').click()
|
|
await retry(async () => {
|
|
expect(await browser.elementByCss('h1').text()).toBe(
|
|
'custom not found page'
|
|
)
|
|
})
|
|
})
|
|
})
|