From ed15673c59b6014daac85894013134d6bc475bdc Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Mon, 13 Sep 2021 12:49:15 -0700 Subject: [PATCH] fixup! Throw error if res is accessed after gSSP returns --- errors/gssp-no-mutating-res | 4 ++-- packages/next/server/render.tsx | 7 ++++--- .../pages/promise/mutate-res-props.js | 21 +++++++++++++++++++ .../getserversideprops/test/index.test.js | 16 +++++++++++++- 4 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 test/integration/getserversideprops/pages/promise/mutate-res-props.js diff --git a/errors/gssp-no-mutating-res b/errors/gssp-no-mutating-res index 30dfc79a764af1..169272ff28a9f6 100644 --- a/errors/gssp-no-mutating-res +++ b/errors/gssp-no-mutating-res @@ -1,8 +1,8 @@ -# Must not access ServerResponse after getServerSideProps() returns +# Must not access ServerResponse after getServerSideProps() resolves #### Why This Error Occurred -`getServerSideProps()` surfaces a `ServerResponse` object through the `res` property of its `context` arg. This object is not intended to be accessed or changed after `getServerSideProps()` returns. +`getServerSideProps()` surfaces a `ServerResponse` object through the `res` property of its `context` arg. This object is not intended to be accessed or changed after `getServerSideProps()` resolves. This is because the framework tries to optimize when items like headers or status codes are flushed to the browser. If they are changed after `getServerSideProps()` completes, we can't guarantee that the changes will work. diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index bf25446d29fac1..664025fd1cb659 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -745,11 +745,12 @@ export async function renderToHTML( let canAccessRes = true let resOrProxy = res if (process.env.NODE_ENV !== 'production') { - resOrProxy = new Proxy(res, { - get: function (obj: ServerResponse, prop: string, receiver: any) { + resOrProxy = new Proxy(res, { + get: function (obj, prop, receiver) { if (!canAccessRes) { throw new Error( - `Must not access ServerResponse after getServerSideProps() returns! https://nextjs.org/docs/messages/gssp-no-mutating-res` + `You should not access 'res' after getServerSideProps resolves.` + + `\nRead more: https://nextjs.org/docs/messages/gssp-no-mutating-res` ) } return Reflect.get(obj, prop, receiver) diff --git a/test/integration/getserversideprops/pages/promise/mutate-res-props.js b/test/integration/getserversideprops/pages/promise/mutate-res-props.js new file mode 100644 index 00000000000000..081bf06fc19a3c --- /dev/null +++ b/test/integration/getserversideprops/pages/promise/mutate-res-props.js @@ -0,0 +1,21 @@ + +export async function getServerSideProps(context) { + return { + props: (async function () { + // Mimic some async work, like getting data from an API + await new Promise((resolve) => setTimeout(resolve)) + context.res.setHeader('test-header', 'this is a test header') + return { + text: 'res', + } + })(), + } +} + +export default ({ text }) => { + return ( + <> +
hello {text}
+ + ) +} diff --git a/test/integration/getserversideprops/test/index.test.js b/test/integration/getserversideprops/test/index.test.js index 3d56e0cacce63b..2544414b073fa6 100644 --- a/test/integration/getserversideprops/test/index.test.js +++ b/test/integration/getserversideprops/test/index.test.js @@ -147,6 +147,12 @@ const expectedManifestRoutes = () => [ `^\\/_next\\/data\\/${escapeRegex(buildId)}\\/promise\\/mutate-res.json$` ), page: '/promise/mutate-res', + }, + { + dataRouteRegex: normalizeRegEx( + `^\\/_next\\/data\\/${escapeRegex(buildId)}\\/promise\\/mutate-res-props.json$` + ), + page: '/promise/mutate-res-props', }, { dataRouteRegex: normalizeRegEx( @@ -721,9 +727,17 @@ const runTests = (dev = false) => { it('should show error for accessing res after gssp returns', async () => { const html = await renderViaHTTP(appPort, '/promise/mutate-res') expect(html).toContain( - 'Must not access ServerResponse after getServerSideProps() returns' + `You should not access 'res' after getServerSideProps resolves` + ) + }) + + it('should show error for accessing res through props promise after gssp returns', async () => { + const html = await renderViaHTTP(appPort, '/promise/mutate-res-props') + expect(html).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')