Warn when mutating res if not streaming (#30284)
In #29010, we started throwing an error if the res was mutated after getServerSideProps() returned. This was to support classic streaming, where it would be possible to accidentally mutate the response headers after they were already sent. However, this change also caught [a few non-streaming cases](https://github.com/vercel/next.js/pull/29010#issuecomment-943482743) that we don't want to break. As such, with this change, we only throw the error if res is mutated after gSSP returns *and* you've opted into using classic streaming. Otherwise, we will only add a warning to the console.
This commit is contained in:
parent
cc4857f3ab
commit
f6c58cb83a
4 changed files with 62 additions and 4 deletions
|
@ -12,6 +12,8 @@ For this reason, accessing the object after this time is disallowed.
|
||||||
|
|
||||||
You can fix this error by moving any access of the `res` object into `getServerSideProps()` itself or any functions that run before `getServerSideProps()` returns.
|
You can fix this error by moving any access of the `res` object into `getServerSideProps()` itself or any functions that run before `getServerSideProps()` returns.
|
||||||
|
|
||||||
|
If you’re using a custom server and running into this problem due to session middleware like `next-session` or `express-session`, try installing the middleware in the server instead of `getServerSideProps()`.
|
||||||
|
|
||||||
### Useful Links
|
### Useful Links
|
||||||
|
|
||||||
- [Data Fetching Docs](https://nextjs.org/docs/basic-features/data-fetching)
|
- [Data Fetching Docs](https://nextjs.org/docs/basic-features/data-fetching)
|
||||||
|
|
|
@ -745,14 +745,20 @@ export async function renderToHTML(
|
||||||
|
|
||||||
let canAccessRes = true
|
let canAccessRes = true
|
||||||
let resOrProxy = res
|
let resOrProxy = res
|
||||||
|
let deferredContent = false
|
||||||
if (process.env.NODE_ENV !== 'production') {
|
if (process.env.NODE_ENV !== 'production') {
|
||||||
resOrProxy = new Proxy<ServerResponse>(res, {
|
resOrProxy = new Proxy<ServerResponse>(res, {
|
||||||
get: function (obj, prop, receiver) {
|
get: function (obj, prop, receiver) {
|
||||||
if (!canAccessRes) {
|
if (!canAccessRes) {
|
||||||
throw new Error(
|
const message =
|
||||||
`You should not access 'res' after getServerSideProps resolves.` +
|
`You should not access 'res' after getServerSideProps resolves.` +
|
||||||
`\nRead more: https://nextjs.org/docs/messages/gssp-no-mutating-res`
|
`\nRead more: https://nextjs.org/docs/messages/gssp-no-mutating-res`
|
||||||
)
|
|
||||||
|
if (deferredContent) {
|
||||||
|
throw new Error(message)
|
||||||
|
} else {
|
||||||
|
warn(message)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return Reflect.get(obj, prop, receiver)
|
return Reflect.get(obj, prop, receiver)
|
||||||
},
|
},
|
||||||
|
@ -792,6 +798,10 @@ export async function renderToHTML(
|
||||||
throw new Error(GSSP_NO_RETURNED_VALUE)
|
throw new Error(GSSP_NO_RETURNED_VALUE)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if ((data as any).props instanceof Promise) {
|
||||||
|
deferredContent = true
|
||||||
|
}
|
||||||
|
|
||||||
const invalidKeys = Object.keys(data).filter(
|
const invalidKeys = Object.keys(data).filter(
|
||||||
(key) => key !== 'props' && key !== 'redirect' && key !== 'notFound'
|
(key) => key !== 'props' && key !== 'redirect' && key !== 'notFound'
|
||||||
)
|
)
|
||||||
|
@ -834,7 +844,7 @@ export async function renderToHTML(
|
||||||
;(renderOpts as any).isRedirect = true
|
;(renderOpts as any).isRedirect = true
|
||||||
}
|
}
|
||||||
|
|
||||||
if ((data as any).props instanceof Promise) {
|
if (deferredContent) {
|
||||||
;(data as any).props = await (data as any).props
|
;(data as any).props = await (data as any).props
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,20 @@
|
||||||
|
let mutatedResNoStreaming
|
||||||
|
|
||||||
|
export async function getServerSideProps(context) {
|
||||||
|
mutatedResNoStreaming = context.res
|
||||||
|
return {
|
||||||
|
props: {
|
||||||
|
text: 'res',
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
export default ({ text }) => {
|
||||||
|
mutatedResNoStreaming.setHeader('test-header', 'this is a test header')
|
||||||
|
|
||||||
|
return (
|
||||||
|
<>
|
||||||
|
<div>hello {text}</div>
|
||||||
|
</>
|
||||||
|
)
|
||||||
|
}
|
|
@ -147,6 +147,14 @@ const expectedManifestRoutes = () => [
|
||||||
),
|
),
|
||||||
page: '/promise/mutate-res',
|
page: '/promise/mutate-res',
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
dataRouteRegex: normalizeRegEx(
|
||||||
|
`^\\/_next\\/data\\/${escapeRegex(
|
||||||
|
buildId
|
||||||
|
)}\\/promise\\/mutate-res-no-streaming.json$`
|
||||||
|
),
|
||||||
|
page: '/promise/mutate-res-no-streaming',
|
||||||
|
},
|
||||||
{
|
{
|
||||||
dataRouteRegex: normalizeRegEx(
|
dataRouteRegex: normalizeRegEx(
|
||||||
`^\\/_next\\/data\\/${escapeRegex(
|
`^\\/_next\\/data\\/${escapeRegex(
|
||||||
|
@ -738,6 +746,19 @@ const runTests = (dev = false) => {
|
||||||
`You should not access 'res' after getServerSideProps resolves`
|
`You should not access 'res' after getServerSideProps resolves`
|
||||||
)
|
)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('should only warn for accessing res if not streaming', async () => {
|
||||||
|
const html = await renderViaHTTP(
|
||||||
|
appPort,
|
||||||
|
'/promise/mutate-res-no-streaming'
|
||||||
|
)
|
||||||
|
expect(html).not.toContain(
|
||||||
|
`You should not access 'res' after getServerSideProps resolves`
|
||||||
|
)
|
||||||
|
expect(stderr).toContain(
|
||||||
|
`You should not access 'res' after getServerSideProps resolves`
|
||||||
|
)
|
||||||
|
})
|
||||||
} else {
|
} else {
|
||||||
it('should not fetch data on mount', async () => {
|
it('should not fetch data on mount', async () => {
|
||||||
const browser = await webdriver(appPort, '/blog/post-100')
|
const browser = await webdriver(appPort, '/blog/post-100')
|
||||||
|
@ -800,6 +821,11 @@ const runTests = (dev = false) => {
|
||||||
const html = await renderViaHTTP(appPort, '/promise/mutate-res')
|
const html = await renderViaHTTP(appPort, '/promise/mutate-res')
|
||||||
expect(html).toMatch(/hello.*?res/)
|
expect(html).toMatch(/hello.*?res/)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('should not warn for accessing res after gssp returns', async () => {
|
||||||
|
const html = await renderViaHTTP(appPort, '/promise/mutate-res')
|
||||||
|
expect(html).toMatch(/hello.*?res/)
|
||||||
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue