From daa9530885a25c91a693534786d5af340340cbff Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Thu, 2 Jun 2022 09:05:00 -0500 Subject: [PATCH 1/9] Split cookies manually instead of using `Headers.raw()` --- packages/hydrogen/package.json | 2 ++ packages/hydrogen/src/entry-server.tsx | 22 +++++++++++++++------- yarn.lock | 7 +++++++ 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/packages/hydrogen/package.json b/packages/hydrogen/package.json index 69508a4435..b22d21e31f 100644 --- a/packages/hydrogen/package.json +++ b/packages/hydrogen/package.json @@ -72,6 +72,7 @@ "@types/node-fetch": "^2.5.9", "@types/react": "^18.0.9", "@types/react-dom": "^18.0.5", + "@types/set-cookie-parser": "^2.4.2", "@types/uuid": "^8.3.4", "@types/ws": "^8.2.0", "babel-loader": "^8.2.2", @@ -117,6 +118,7 @@ "path-to-regexp": "^6.2.0", "react-error-boundary": "^3.1.3", "react-helmet-async": "^1.2.3", + "set-cookie-parser": "^2.4.8", "uuid": "^8.3.2", "vite-plugin-inspect": "^0.3.6", "web-streams-polyfill": "^3.2.0", diff --git a/packages/hydrogen/src/entry-server.tsx b/packages/hydrogen/src/entry-server.tsx index b310ebc3e0..0fa570bbb9 100644 --- a/packages/hydrogen/src/entry-server.tsx +++ b/packages/hydrogen/src/entry-server.tsx @@ -49,6 +49,7 @@ import {ServerAnalyticsRoute} from './foundation/Analytics/ServerAnalyticsRoute. import {getSyncSessionApi} from './foundation/session/session'; import {parseJSON} from './utilities/parse'; import {htmlEncode} from './utilities'; +import {splitCookiesString} from 'set-cookie-parser'; declare global { // This is provided by a Vite plugin @@ -717,12 +718,19 @@ function handleFetchResponseInNode( return fetchResponsePromise; } -// From fetch Headers to Node Response +/** + * Convert Headers to outgoing Node.js headers. + * Specifically, parse set-cookie headers to split them properly as separate + * `set-cookie` headers rather than a single, combined header. + */ function setNodeHeaders(headers: Headers, nodeResponse: ServerResponse) { - // Headers.raw is only implemented in node-fetch, which is used by Hydrogen in dev and prod. - // It is the only way for now to access `set-cookie` header as an array. - // https://github.com/Shopify/hydrogen/issues/1228 - Object.entries((headers as any).raw()).forEach(([key, value]) => - nodeResponse.setHeader(key, value as string) - ); + for (const [key, value] of headers.entries()) { + nodeResponse.setHeader(key, value); + if (key.toLowerCase() === 'set-cookie') { + const cookies = splitCookiesString(value); + cookies.forEach((cookie) => { + nodeResponse.setHeader('set-cookies', cookie); + }); + } + } } diff --git a/yarn.lock b/yarn.lock index 37cd45e316..ed097dc5ae 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2664,6 +2664,13 @@ resolved "https://registry.yarnpkg.com/@types/semver/-/semver-6.2.3.tgz#5798ecf1bec94eaa64db39ee52808ec0693315aa" integrity sha512-KQf+QAMWKMrtBMsB8/24w53tEsxllMj6TuA80TT/5igJalLI/zm0L3oXRbIAl4Ohfc85gyHX/jhMwsVkmhLU4A== +"@types/set-cookie-parser@^2.4.2": + version "2.4.2" + resolved "https://registry.yarnpkg.com/@types/set-cookie-parser/-/set-cookie-parser-2.4.2.tgz#b6a955219b54151bfebd4521170723df5e13caad" + integrity sha512-fBZgytwhYAUkj/jC/FAV4RQ5EerRup1YQsXQCh8rZfiHkc4UahC192oH0smGwsXol3cL3A5oETuAHeQHmhXM4w== + dependencies: + "@types/node" "*" + "@types/stack-trace@0.0.29": version "0.0.29" resolved "https://registry.yarnpkg.com/@types/stack-trace/-/stack-trace-0.0.29.tgz#eb7a7c60098edb35630ed900742a5ecb20cfcb4d" From 3b5a2fc34a707c1757bfcbebab4a96968855c47d Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Thu, 2 Jun 2022 09:05:16 -0500 Subject: [PATCH 2/9] Add Node v18 to the testing matrix --- .github/workflows/tests_and_lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests_and_lint.yml b/.github/workflows/tests_and_lint.yml index e848546e7b..37ffb08459 100644 --- a/.github/workflows/tests_and_lint.yml +++ b/.github/workflows/tests_and_lint.yml @@ -21,7 +21,7 @@ jobs: strategy: matrix: os: [windows-latest, ubuntu-latest] - node-version: [16.x, 17.x] + node-version: [16.x, 17.x, 18.x] name: OS ${{ matrix.os }} / NodeJS ${{ matrix.node-version }} From 628e5d8afa2b8af554c2c4efb2150e1f261dfd07 Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Thu, 2 Jun 2022 09:05:36 -0500 Subject: [PATCH 3/9] Add changeset --- .changeset/sharp-rice-marry.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/sharp-rice-marry.md diff --git a/.changeset/sharp-rice-marry.md b/.changeset/sharp-rice-marry.md new file mode 100644 index 0000000000..a09cfce49c --- /dev/null +++ b/.changeset/sharp-rice-marry.md @@ -0,0 +1,5 @@ +--- +'@shopify/hydrogen': patch +--- + +Properly support Node v18 From 10569ef017d554beb64c2188bc3ace8be46592b9 Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Thu, 2 Jun 2022 09:20:30 -0500 Subject: [PATCH 4/9] Fix types, logic, and bundling issues --- packages/hydrogen/src/entry-server.tsx | 8 +++----- .../src/framework/plugins/vite-plugin-hydrogen-config.ts | 2 ++ packages/hydrogen/tsconfig.cjs.json | 2 +- packages/hydrogen/tsconfig.json | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/hydrogen/src/entry-server.tsx b/packages/hydrogen/src/entry-server.tsx index 0fa570bbb9..9e6e87b76f 100644 --- a/packages/hydrogen/src/entry-server.tsx +++ b/packages/hydrogen/src/entry-server.tsx @@ -725,12 +725,10 @@ function handleFetchResponseInNode( */ function setNodeHeaders(headers: Headers, nodeResponse: ServerResponse) { for (const [key, value] of headers.entries()) { - nodeResponse.setHeader(key, value); if (key.toLowerCase() === 'set-cookie') { - const cookies = splitCookiesString(value); - cookies.forEach((cookie) => { - nodeResponse.setHeader('set-cookies', cookie); - }); + nodeResponse.setHeader('set-cookie', splitCookiesString(value)); + } else { + nodeResponse.setHeader(key, value); } } } diff --git a/packages/hydrogen/src/framework/plugins/vite-plugin-hydrogen-config.ts b/packages/hydrogen/src/framework/plugins/vite-plugin-hydrogen-config.ts index 5cae189466..5d1a8a66b7 100644 --- a/packages/hydrogen/src/framework/plugins/vite-plugin-hydrogen-config.ts +++ b/packages/hydrogen/src/framework/plugins/vite-plugin-hydrogen-config.ts @@ -94,6 +94,8 @@ export default () => { 'react-server-dom-vite/client-proxy', // https://github.com/vitejs/vite/issues/6215 'react/jsx-runtime', + // https://github.com/nfriedly/set-cookie-parser/issues/50 + 'set-cookie-parser', ], }, diff --git a/packages/hydrogen/tsconfig.cjs.json b/packages/hydrogen/tsconfig.cjs.json index 764a6f8e0c..6e4d3d9823 100644 --- a/packages/hydrogen/tsconfig.cjs.json +++ b/packages/hydrogen/tsconfig.cjs.json @@ -6,7 +6,7 @@ "outDir": "./dist/node", "module": "commonjs", "moduleResolution": "node", - "lib": ["ESNext", "DOM"], + "lib": ["ESNext", "DOM", "DOM.Iterable"], "esModuleInterop": true, "types": [] } diff --git a/packages/hydrogen/tsconfig.json b/packages/hydrogen/tsconfig.json index 5f5cad2f87..1f1b08e1c2 100644 --- a/packages/hydrogen/tsconfig.json +++ b/packages/hydrogen/tsconfig.json @@ -8,7 +8,7 @@ "outDir": "./dist/esnext", "module": "esnext", "moduleResolution": "node", - "lib": ["ESNext", "DOM"], + "lib": ["ESNext", "DOM", "DOM.Iterable"], "types": ["jest", "@shopify/react-testing/matchers"], "strict": true, "noUnusedLocals": true, From c6dd1558fa8cd789db41330f3767cb4e642fe2ae Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Thu, 2 Jun 2022 09:45:45 -0500 Subject: [PATCH 5/9] Add support for sending ReadableStream to node responses --- packages/hydrogen/src/entry-server.tsx | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/hydrogen/src/entry-server.tsx b/packages/hydrogen/src/entry-server.tsx index 9e6e87b76f..c52eb885a4 100644 --- a/packages/hydrogen/src/entry-server.tsx +++ b/packages/hydrogen/src/entry-server.tsx @@ -708,10 +708,20 @@ function handleFetchResponseInNode( nodeResponse.statusCode = response.status; if (response.body) { - nodeResponse.write(response.body); + if (response.body instanceof ReadableStream) { + const reader = response.body.getReader(); + reader.read().then(function write({done, value}) { + value && nodeResponse.write(value); + !done && reader.read().then(write); + done && nodeResponse.end(); + }); + } else { + nodeResponse.write(response.body); + nodeResponse.end(); + } + } else { + nodeResponse.end(); } - - nodeResponse.end(); }); } From a27507a7489234fe4530e0cc93870fc6f4d02307 Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Mon, 6 Jun 2022 12:43:56 +0900 Subject: [PATCH 6/9] Update set-cookie-parser to enhance tree-shaking in workers --- packages/hydrogen/package.json | 2 +- yarn.lock | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/hydrogen/package.json b/packages/hydrogen/package.json index b22d21e31f..6cb98299b6 100644 --- a/packages/hydrogen/package.json +++ b/packages/hydrogen/package.json @@ -118,7 +118,7 @@ "path-to-regexp": "^6.2.0", "react-error-boundary": "^3.1.3", "react-helmet-async": "^1.2.3", - "set-cookie-parser": "^2.4.8", + "set-cookie-parser": "^2.5.0", "uuid": "^8.3.2", "vite-plugin-inspect": "^0.3.6", "web-streams-polyfill": "^3.2.0", diff --git a/yarn.lock b/yarn.lock index ed097dc5ae..441fc584c9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11249,6 +11249,11 @@ set-cookie-parser@^2.4.8: resolved "https://registry.yarnpkg.com/set-cookie-parser/-/set-cookie-parser-2.4.8.tgz#d0da0ed388bc8f24e706a391f9c9e252a13c58b2" integrity sha512-edRH8mBKEWNVIVMKejNnuJxleqYE/ZSdcT8/Nem9/mmosx12pctd80s2Oy00KNZzrogMZS5mauK2/ymL1bvlvg== +set-cookie-parser@^2.5.0: + version "2.5.0" + resolved "https://registry.yarnpkg.com/set-cookie-parser/-/set-cookie-parser-2.5.0.tgz#96b59525e1362c94335c3c761100bb6e8f2da4b0" + integrity sha512-cHMAtSXilfyBePduZEBVPTCftTQWz6ehWJD5YNUg4mqvRosrrjKbo4WS8JkB0/RxonMoohHm7cOGH60mDkRQ9w== + set-value@^2.0.0, set-value@^2.0.1: version "2.0.1" resolved "https://registry.yarnpkg.com/set-value/-/set-value-2.0.1.tgz#a18d40530e6f07de4228c7defe4227af8cad005b" From b02fad0d791d72d097007947796933489814f2ca Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Mon, 6 Jun 2022 09:28:45 -0500 Subject: [PATCH 7/9] Update packages/hydrogen/src/entry-server.tsx Co-authored-by: Fran Dios --- packages/hydrogen/src/entry-server.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/hydrogen/src/entry-server.tsx b/packages/hydrogen/src/entry-server.tsx index c52eb885a4..f46867a51a 100644 --- a/packages/hydrogen/src/entry-server.tsx +++ b/packages/hydrogen/src/entry-server.tsx @@ -736,7 +736,7 @@ function handleFetchResponseInNode( function setNodeHeaders(headers: Headers, nodeResponse: ServerResponse) { for (const [key, value] of headers.entries()) { if (key.toLowerCase() === 'set-cookie') { - nodeResponse.setHeader('set-cookie', splitCookiesString(value)); + nodeResponse.setHeader(key, splitCookiesString(value)); } else { nodeResponse.setHeader(key, value); } From 88e0589e52d7cf44727537aa7f5661c5fde77527 Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Mon, 6 Jun 2022 09:34:08 -0500 Subject: [PATCH 8/9] Use existing bufferReadableStream utility --- packages/hydrogen/src/entry-server.tsx | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/hydrogen/src/entry-server.tsx b/packages/hydrogen/src/entry-server.tsx index f46867a51a..dd89a0ad15 100644 --- a/packages/hydrogen/src/entry-server.tsx +++ b/packages/hydrogen/src/entry-server.tsx @@ -709,12 +709,9 @@ function handleFetchResponseInNode( if (response.body) { if (response.body instanceof ReadableStream) { - const reader = response.body.getReader(); - reader.read().then(function write({done, value}) { - value && nodeResponse.write(value); - !done && reader.read().then(write); - done && nodeResponse.end(); - }); + bufferReadableStream(response.body.getReader(), (chunk) => { + nodeResponse.write(chunk); + }).then(() => nodeResponse.end()); } else { nodeResponse.write(response.body); nodeResponse.end(); From 6a01ab1924dff8711a2f041821c0e84a27000ff4 Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Mon, 6 Jun 2022 09:34:21 -0500 Subject: [PATCH 9/9] Don't run tests on Node 17 anymore --- .github/workflows/tests_and_lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests_and_lint.yml b/.github/workflows/tests_and_lint.yml index 37ffb08459..edcc818bb1 100644 --- a/.github/workflows/tests_and_lint.yml +++ b/.github/workflows/tests_and_lint.yml @@ -21,7 +21,7 @@ jobs: strategy: matrix: os: [windows-latest, ubuntu-latest] - node-version: [16.x, 17.x, 18.x] + node-version: [16.x, 18.x] name: OS ${{ matrix.os }} / NodeJS ${{ matrix.node-version }}