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

Warn when mutating res if not streaming #30284

Merged
merged 2 commits into from Oct 26, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions errors/gssp-no-mutating-res.md
Expand Up @@ -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.

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

- [Data Fetching Docs](https://nextjs.org/docs/basic-features/data-fetching)
18 changes: 14 additions & 4 deletions packages/next/server/render.tsx
Expand Up @@ -745,14 +745,20 @@ export async function renderToHTML(

let canAccessRes = true
let resOrProxy = res
let deferredContent = false
if (process.env.NODE_ENV !== 'production') {
resOrProxy = new Proxy<ServerResponse>(res, {
get: function (obj, prop, receiver) {
if (!canAccessRes) {
throw new Error(
const message =
`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)
},
Expand Down Expand Up @@ -792,6 +798,10 @@ export async function renderToHTML(
throw new Error(GSSP_NO_RETURNED_VALUE)
}

if ((data as any).props instanceof Promise) {
deferredContent = true
}

const invalidKeys = Object.keys(data).filter(
(key) => key !== 'props' && key !== 'redirect' && key !== 'notFound'
)
Expand Down Expand Up @@ -834,7 +844,7 @@ export async function renderToHTML(
;(renderOpts as any).isRedirect = true
}

if ((data as any).props instanceof Promise) {
if (deferredContent) {
;(data as any).props = await (data as any).props
}

Expand Down
@@ -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>
</>
)
}
26 changes: 26 additions & 0 deletions test/integration/getserversideprops/test/index.test.js
Expand Up @@ -147,6 +147,14 @@ const expectedManifestRoutes = () => [
),
page: '/promise/mutate-res',
},
{
dataRouteRegex: normalizeRegEx(
`^\\/_next\\/data\\/${escapeRegex(
buildId
)}\\/promise\\/mutate-res-no-streaming.json$`
),
page: '/promise/mutate-res-no-streaming',
},
{
dataRouteRegex: normalizeRegEx(
`^\\/_next\\/data\\/${escapeRegex(
Expand Down Expand Up @@ -738,6 +746,19 @@ const runTests = (dev = false) => {
`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 {
it('should not fetch data on mount', async () => {
const browser = await webdriver(appPort, '/blog/post-100')
Expand Down Expand Up @@ -800,6 +821,11 @@ const runTests = (dev = false) => {
const html = await renderViaHTTP(appPort, '/promise/mutate-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/)
})
}
}

Expand Down