From 34735595b7b1b02431105b2fe72781244775560c Mon Sep 17 00:00:00 2001 From: takurinton Date: Sun, 18 Sep 2022 16:11:17 +0900 Subject: [PATCH 1/9] Enable fetching external resources using native fetch API in Node.js version 18 later --- packages/wmr/src/lib/prerender.js | 21 +++++++++++++++- .../index.html | 10 ++++++++ .../index.js | 4 +++ packages/wmr/test/production.test.js | 25 +++++++++++++++++++ 4 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 packages/wmr/test/fixtures/prerender-external-resource-fetch/index.html create mode 100644 packages/wmr/test/fixtures/prerender-external-resource-fetch/index.js diff --git a/packages/wmr/src/lib/prerender.js b/packages/wmr/src/lib/prerender.js index 9f5297ca1..72fe519d8 100644 --- a/packages/wmr/src/lib/prerender.js +++ b/packages/wmr/src/lib/prerender.js @@ -97,7 +97,26 @@ async function workerCode({ cwd, out, publicPath, customRoutes }) { globalThis.wmr = { ssr: { head } }; // @ts-ignore - globalThis.fetch = async url => { + globalThis.fetch = async (url, options) => { + const pattern = /^https?:\/\/[\w/:%#\\$&\\?\\(\\)~\\.=\\+\\-]+/; + const isExternalUrl = pattern.test(String(url)); + if (isExternalUrl) { + const nodeVersion = process.version; + const isNodeVersionOver18 = Number(nodeVersion.split('.')[0].slice(1)) >= 18; + // Use native fetch if Node.js version over 18 when `prerender` + if (isNodeVersionOver18) { + // TODO: Solve "RangeError: Maximum call stack size exceeded" + // MEMO: fetch is called recursively... + return fetch(url, options).then(res => ({ + text: () => res.text(), + json: () => res.text().then(JSON.parse) + })); + } + throw new Error( + `External links are not supported in your current Node.js version ${nodeVersion} (try v18 or later).\n ${url}` + ); + } + const text = () => fs.readFile(`${out}/${String(url).replace(/^\//, '')}`, 'utf-8'); return { text, json: () => text().then(JSON.parse) }; }; diff --git a/packages/wmr/test/fixtures/prerender-external-resource-fetch/index.html b/packages/wmr/test/fixtures/prerender-external-resource-fetch/index.html new file mode 100644 index 000000000..d2c9fb7d0 --- /dev/null +++ b/packages/wmr/test/fixtures/prerender-external-resource-fetch/index.html @@ -0,0 +1,10 @@ + + + + + default title + + + + + diff --git a/packages/wmr/test/fixtures/prerender-external-resource-fetch/index.js b/packages/wmr/test/fixtures/prerender-external-resource-fetch/index.js new file mode 100644 index 000000000..e6a8f3752 --- /dev/null +++ b/packages/wmr/test/fixtures/prerender-external-resource-fetch/index.js @@ -0,0 +1,4 @@ +export async function prerender() { + const html = await fetch('https://preactjs.com').then(res => res.text()); + return { html, links: ['/'] }; +} diff --git a/packages/wmr/test/production.test.js b/packages/wmr/test/production.test.js index c26226288..b28daf7b4 100644 --- a/packages/wmr/test/production.test.js +++ b/packages/wmr/test/production.test.js @@ -889,6 +889,31 @@ describe('production', () => { const index = await fs.readFile(indexHtml, 'utf8'); expect(index).toMatch(/# hello world/); }); + + it('should not support fetching resources from external link prerender', async () => { + await loadFixture('prerender-external-resource-fetch', env); + instance = await runWmr(env.tmp.path, 'build', '--prerender'); + const code = await instance.done; + + const nodeVersion = process.version; + const isNodeVersionOver18 = Number(nodeVersion.split('.')[0].slice(1)) >= 18; + if (isNodeVersionOver18) { + // when workflow of wmr runtime uses Node.js 18 + // Until then check locally. + expect(code).toBe(0); + const indexHtml = path.join(env.tmp.path, 'dist', 'index.html'); + const index = await fs.readFile(indexHtml, 'utf8'); + expect(index).toMatch(/Preact/); + } else { + // Throw error when Node.js version under 17 + await withLog(instance.output, async () => { + expect(code).toBe(1); + expect(instance.output.join('\n')).toMatch( + /Error: External links are not supported in your current Node.js version/ + ); + }); + } + }); }); describe('Code Splitting', () => { From c9d9d56d052b4d133fb1fd53ff6c7d2a837dcace Mon Sep 17 00:00:00 2001 From: takurinton Date: Mon, 19 Sep 2022 04:51:14 +0900 Subject: [PATCH 2/9] fix: remove fetch after redefinition to avoid infinite loop --- packages/wmr/src/lib/prerender.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/wmr/src/lib/prerender.js b/packages/wmr/src/lib/prerender.js index 72fe519d8..cffd38afb 100644 --- a/packages/wmr/src/lib/prerender.js +++ b/packages/wmr/src/lib/prerender.js @@ -96,18 +96,24 @@ async function workerCode({ cwd, out, publicPath, customRoutes }) { let head = { lang: '', title: '', elements: new Set() }; globalThis.wmr = { ssr: { head } }; + const _fetch = fetch; + // @ts-ignore + delete globalThis.fetch; // @ts-ignore globalThis.fetch = async (url, options) => { + let isRequested = false; const pattern = /^https?:\/\/[\w/:%#\\$&\\?\\(\\)~\\.=\\+\\-]+/; const isExternalUrl = pattern.test(String(url)); if (isExternalUrl) { const nodeVersion = process.version; const isNodeVersionOver18 = Number(nodeVersion.split('.')[0].slice(1)) >= 18; + if (isRequested) { + return; + } // Use native fetch if Node.js version over 18 when `prerender` if (isNodeVersionOver18) { - // TODO: Solve "RangeError: Maximum call stack size exceeded" - // MEMO: fetch is called recursively... - return fetch(url, options).then(res => ({ + isRequested = true; + return _fetch(url, options).then(res => ({ text: () => res.text(), json: () => res.text().then(JSON.parse) })); From aa0818dc236cc51d9ee04189aa7a3c3cbf4ab011 Mon Sep 17 00:00:00 2001 From: takurinton Date: Mon, 19 Sep 2022 05:05:36 +0900 Subject: [PATCH 3/9] fix: when Node.js version is only over 18 define `_fetch` instead native `fetch` --- packages/wmr/src/lib/prerender.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/wmr/src/lib/prerender.js b/packages/wmr/src/lib/prerender.js index cffd38afb..63d2088b0 100644 --- a/packages/wmr/src/lib/prerender.js +++ b/packages/wmr/src/lib/prerender.js @@ -96,7 +96,13 @@ async function workerCode({ cwd, out, publicPath, customRoutes }) { let head = { lang: '', title: '', elements: new Set() }; globalThis.wmr = { ssr: { head } }; - const _fetch = fetch; + let _fetch; + const nodeVersion = process.version; + const isNodeVersionOver18 = Number(nodeVersion.split('.')[0].slice(1)) >= 18; + // if Node.js version is under 18, fetch is undefined + if (isNodeVersionOver18) { + _fetch = fetch; + } // @ts-ignore delete globalThis.fetch; // @ts-ignore @@ -105,8 +111,6 @@ async function workerCode({ cwd, out, publicPath, customRoutes }) { const pattern = /^https?:\/\/[\w/:%#\\$&\\?\\(\\)~\\.=\\+\\-]+/; const isExternalUrl = pattern.test(String(url)); if (isExternalUrl) { - const nodeVersion = process.version; - const isNodeVersionOver18 = Number(nodeVersion.split('.')[0].slice(1)) >= 18; if (isRequested) { return; } From 8acd946ab67a920fa5b319a62b512f8a09401498 Mon Sep 17 00:00:00 2001 From: takurinton Date: Mon, 19 Sep 2022 13:31:48 +0900 Subject: [PATCH 4/9] add: changeset --- .changeset/young-guests-tap.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/young-guests-tap.md diff --git a/.changeset/young-guests-tap.md b/.changeset/young-guests-tap.md new file mode 100644 index 000000000..9ff143c79 --- /dev/null +++ b/.changeset/young-guests-tap.md @@ -0,0 +1,5 @@ +--- +'wmr': minor +--- + +Add fetching external resources using native fetch API when Node.js version 18 later From 74f90a9b44a67f1651f7409fd603a70dbe73ca16 Mon Sep 17 00:00:00 2001 From: takurinton Date: Mon, 19 Sep 2022 14:04:17 +0900 Subject: [PATCH 5/9] fix --- packages/wmr/src/lib/prerender.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/wmr/src/lib/prerender.js b/packages/wmr/src/lib/prerender.js index 63d2088b0..ccb815578 100644 --- a/packages/wmr/src/lib/prerender.js +++ b/packages/wmr/src/lib/prerender.js @@ -96,10 +96,11 @@ async function workerCode({ cwd, out, publicPath, customRoutes }) { let head = { lang: '', title: '', elements: new Set() }; globalThis.wmr = { ssr: { head } }; + // Define `_fetch` to avoid infinite loop in globalThis.fetch. let _fetch; const nodeVersion = process.version; const isNodeVersionOver18 = Number(nodeVersion.split('.')[0].slice(1)) >= 18; - // if Node.js version is under 18, fetch is undefined + // Native fetch is only available in Node.js 18 later. if (isNodeVersionOver18) { _fetch = fetch; } @@ -107,16 +108,11 @@ async function workerCode({ cwd, out, publicPath, customRoutes }) { delete globalThis.fetch; // @ts-ignore globalThis.fetch = async (url, options) => { - let isRequested = false; const pattern = /^https?:\/\/[\w/:%#\\$&\\?\\(\\)~\\.=\\+\\-]+/; const isExternalUrl = pattern.test(String(url)); if (isExternalUrl) { - if (isRequested) { - return; - } - // Use native fetch if Node.js version over 18 when `prerender` + // Use native fetch if Node.js version 18 later when `prerender` if (isNodeVersionOver18) { - isRequested = true; return _fetch(url, options).then(res => ({ text: () => res.text(), json: () => res.text().then(JSON.parse) From 54e5d2a9c2d6fb3cedbba683ce3bd9e7a4f0946c Mon Sep 17 00:00:00 2001 From: takurinton Date: Thu, 22 Sep 2022 14:46:44 +0900 Subject: [PATCH 6/9] fix: globalThis.fetch only calls when fetch is undefined and isExternalLink to isLocalFile --- packages/wmr/src/lib/prerender.js | 39 ++++++------------- .../prerender-resource-fetch/index.js | 2 +- packages/wmr/test/production.test.js | 2 +- 3 files changed, 13 insertions(+), 30 deletions(-) diff --git a/packages/wmr/src/lib/prerender.js b/packages/wmr/src/lib/prerender.js index ccb815578..f64f325c4 100644 --- a/packages/wmr/src/lib/prerender.js +++ b/packages/wmr/src/lib/prerender.js @@ -96,36 +96,19 @@ async function workerCode({ cwd, out, publicPath, customRoutes }) { let head = { lang: '', title: '', elements: new Set() }; globalThis.wmr = { ssr: { head } }; - // Define `_fetch` to avoid infinite loop in globalThis.fetch. - let _fetch; - const nodeVersion = process.version; - const isNodeVersionOver18 = Number(nodeVersion.split('.')[0].slice(1)) >= 18; - // Native fetch is only available in Node.js 18 later. - if (isNodeVersionOver18) { - _fetch = fetch; - } - // @ts-ignore - delete globalThis.fetch; - // @ts-ignore - globalThis.fetch = async (url, options) => { - const pattern = /^https?:\/\/[\w/:%#\\$&\\?\\(\\)~\\.=\\+\\-]+/; - const isExternalUrl = pattern.test(String(url)); - if (isExternalUrl) { - // Use native fetch if Node.js version 18 later when `prerender` - if (isNodeVersionOver18) { - return _fetch(url, options).then(res => ({ - text: () => res.text(), - json: () => res.text().then(JSON.parse) - })); + if (globalThis.fetch === undefined) { + // @ts-ignore + globalThis.fetch = async url => { + const pattern = /^\//; + const isLocalFile = pattern.test(String(url)); + if (isLocalFile) { + const text = () => fs.readFile(`${out}/${String(url).replace(/^\//, '')}`, 'utf-8'); + return { text, json: () => text().then(JSON.parse) }; } - throw new Error( - `External links are not supported in your current Node.js version ${nodeVersion} (try v18 or later).\n ${url}` - ); - } - const text = () => fs.readFile(`${out}/${String(url).replace(/^\//, '')}`, 'utf-8'); - return { text, json: () => text().then(JSON.parse) }; - }; + console.warn(`fetch is not implemented in Node.js, please upgrade to Node.js 18 or above`); + }; + } // Prevent Rollup from transforming `import()` here. const $import = new Function('s', 'return import(s)'); diff --git a/packages/wmr/test/fixtures/prerender-resource-fetch/index.js b/packages/wmr/test/fixtures/prerender-resource-fetch/index.js index f0ec977f0..765a62eae 100644 --- a/packages/wmr/test/fixtures/prerender-resource-fetch/index.js +++ b/packages/wmr/test/fixtures/prerender-resource-fetch/index.js @@ -1,4 +1,4 @@ export async function prerender() { - const md = await fetch('content.md').then(res => res.text()); + const md = await fetch('/content.md').then(res => res.text()); return { html: md, links: ['/'] }; } diff --git a/packages/wmr/test/production.test.js b/packages/wmr/test/production.test.js index b28daf7b4..999cc422a 100644 --- a/packages/wmr/test/production.test.js +++ b/packages/wmr/test/production.test.js @@ -909,7 +909,7 @@ describe('production', () => { await withLog(instance.output, async () => { expect(code).toBe(1); expect(instance.output.join('\n')).toMatch( - /Error: External links are not supported in your current Node.js version/ + /fetch is not implemented in Node.js, please upgrade to Node.js 18 or above/ ); }); } From 27def4e6c2b2714b9117ed50535755b626333324 Mon Sep 17 00:00:00 2001 From: takurinton Date: Thu, 22 Sep 2022 21:46:46 +0900 Subject: [PATCH 7/9] fix: override with own implementation if you want to retrieve local files in Node.js 18 later --- packages/wmr/src/lib/prerender.js | 36 +++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/packages/wmr/src/lib/prerender.js b/packages/wmr/src/lib/prerender.js index f64f325c4..ad2829198 100644 --- a/packages/wmr/src/lib/prerender.js +++ b/packages/wmr/src/lib/prerender.js @@ -96,17 +96,41 @@ async function workerCode({ cwd, out, publicPath, customRoutes }) { let head = { lang: '', title: '', elements: new Set() }; globalThis.wmr = { ssr: { head } }; + async function localFetcher(url) { + const text = () => fs.readFile(`${out}/${String(url).replace(/^\//, '')}`, 'utf-8'); + return { text, json: () => text().then(JSON.parse) }; + } + + const pattern = /^\//; if (globalThis.fetch === undefined) { + // When Node.js version under 17, native fetch API undefined + // So it will give a warning when the syntax to retrieve the file and the url is passed // @ts-ignore globalThis.fetch = async url => { - const pattern = /^\//; - const isLocalFile = pattern.test(String(url)); - if (isLocalFile) { - const text = () => fs.readFile(`${out}/${String(url).replace(/^\//, '')}`, 'utf-8'); - return { text, json: () => text().then(JSON.parse) }; + if (pattern.test(String(url))) { + return localFetcher(url); + } + + console.warn(`fetch is not defined in Node.js version under 17, please upgrade to Node.js version 18 later`); + }; + } else { + // At that time, implement using reassignment to avoid entering an infinite loop. + const fetcher = fetch; + // @ts-ignore + delete globalThis.fetch; + + // When Node.js version 18 later, native fetch API is defined + // Override with own implementation if you want to retrieve local files in Node.js 18 later + // ref https://github.com/preactjs/wmr/pull/935#discussion_r977168334 + // @ts-ignore + globalThis.fetch = async (url, options) => { + if (pattern.test(String(url))) { + return localFetcher(url); } - console.warn(`fetch is not implemented in Node.js, please upgrade to Node.js 18 or above`); + // When fetching an external resource, it returns the native fetch API + // though `fetcher` function is an alias created to avoid infinite loops + return fetcher(url, options); }; } From f1a51ddf97ef8b79d62ddfc45b6ab40c5dce9162 Mon Sep 17 00:00:00 2001 From: takurinton Date: Thu, 22 Sep 2022 21:46:56 +0900 Subject: [PATCH 8/9] fix: tests --- packages/wmr/test/production.test.js | 43 +++++++++++++++++----------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/packages/wmr/test/production.test.js b/packages/wmr/test/production.test.js index 999cc422a..bbd673644 100644 --- a/packages/wmr/test/production.test.js +++ b/packages/wmr/test/production.test.js @@ -890,30 +890,39 @@ describe('production', () => { expect(index).toMatch(/# hello world/); }); - it('should not support fetching resources from external link prerender', async () => { - await loadFixture('prerender-external-resource-fetch', env); - instance = await runWmr(env.tmp.path, 'build', '--prerender'); - const code = await instance.done; + // Whether or not to run the test to fetch external resources depends on whether or not a Node.js version 18 later + const isNodeVersionOver18 = Number(process.version.split('.')[0].slice(1)) >= 18; + // Even if you are using Node.js v18 or higher, you will get the error `globalThis.fetch is undefined` here, so get the version from process.version + // if (globalThis.fetch === undefined) + if (!isNodeVersionOver18) { + it.skip('skip should not support fetching resources from external link prerender when Node.js v18 later', () => {}); + it('should not support fetching resources from external link prerender when Node.js befor v17', async () => { + await loadFixture('prerender-external-resource-fetch', env); + instance = await runWmr(env.tmp.path, 'build', '--prerender'); + const code = await instance.done; + + await withLog(instance.output, async () => { + expect(code).toBe(1); + expect(instance.output.join('\n')).toMatch( + /fetch is not defined in Node.js version under 17, please upgrade to Node.js version 18 later/ + ); + }); + }); + } else { + it.skip('skip should not support fetching resources from external link prerender when Node.js befor v17', () => {}); + it('should not support fetching resources from external link prerender when Node.js v18 later', async () => { + await loadFixture('prerender-external-resource-fetch', env); + instance = await runWmr(env.tmp.path, 'build', '--prerender'); + const code = await instance.done; - const nodeVersion = process.version; - const isNodeVersionOver18 = Number(nodeVersion.split('.')[0].slice(1)) >= 18; - if (isNodeVersionOver18) { // when workflow of wmr runtime uses Node.js 18 // Until then check locally. expect(code).toBe(0); const indexHtml = path.join(env.tmp.path, 'dist', 'index.html'); const index = await fs.readFile(indexHtml, 'utf8'); expect(index).toMatch(/Preact/); - } else { - // Throw error when Node.js version under 17 - await withLog(instance.output, async () => { - expect(code).toBe(1); - expect(instance.output.join('\n')).toMatch( - /fetch is not implemented in Node.js, please upgrade to Node.js 18 or above/ - ); - }); - } - }); + }); + } }); describe('Code Splitting', () => { From 63954dd66d4fca9cb90c99de68943552724e5214 Mon Sep 17 00:00:00 2001 From: takurinton Date: Fri, 23 Sep 2022 06:39:42 +0900 Subject: [PATCH 9/9] fix: test names --- packages/wmr/test/production.test.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/wmr/test/production.test.js b/packages/wmr/test/production.test.js index bbd673644..011c1395f 100644 --- a/packages/wmr/test/production.test.js +++ b/packages/wmr/test/production.test.js @@ -891,12 +891,12 @@ describe('production', () => { }); // Whether or not to run the test to fetch external resources depends on whether or not a Node.js version 18 later - const isNodeVersionOver18 = Number(process.version.split('.')[0].slice(1)) >= 18; + const isNodeVersionUnder18 = Number(process.version.split('.')[0].slice(1)) < 18; // Even if you are using Node.js v18 or higher, you will get the error `globalThis.fetch is undefined` here, so get the version from process.version // if (globalThis.fetch === undefined) - if (!isNodeVersionOver18) { - it.skip('skip should not support fetching resources from external link prerender when Node.js v18 later', () => {}); - it('should not support fetching resources from external link prerender when Node.js befor v17', async () => { + if (isNodeVersionUnder18) { + it.skip('skip should support fetching resources from external links during prerender on Node.js v18 later', () => {}); + it('should not support fetching resources from external links during prerender on Node.js < v18', async () => { await loadFixture('prerender-external-resource-fetch', env); instance = await runWmr(env.tmp.path, 'build', '--prerender'); const code = await instance.done; @@ -909,8 +909,8 @@ describe('production', () => { }); }); } else { - it.skip('skip should not support fetching resources from external link prerender when Node.js befor v17', () => {}); - it('should not support fetching resources from external link prerender when Node.js v18 later', async () => { + it.skip('skip should not support fetching resources from external links during prerender on Node.js < v18', () => {}); + it('should support fetching resources from external links during prerender on Node.js v18 later', async () => { await loadFixture('prerender-external-resource-fetch', env); instance = await runWmr(env.tmp.path, 'build', '--prerender'); const code = await instance.done;