From 40da2d557a34be802f2ef7deea806201d28e87ed Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 23 May 2024 15:59:53 +0200 Subject: [PATCH 1/5] fix: Resolve re-exports of external modules --- hook.js | 24 +++++++++++++++++-- .../some-external-module/index.js | 1 + .../some-external-module/package.json | 8 +++++++ .../node_modules/some-external-module/sub.js | 1 + test/fixtures/re-export-star-external.mjs | 1 + .../sub-directory/re-export-star-external.mjs | 1 + test/hook/re-export-star-module.mjs | 17 +++++++++++++ 7 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/node_modules/some-external-module/index.js create mode 100644 test/fixtures/node_modules/some-external-module/package.json create mode 100644 test/fixtures/node_modules/some-external-module/sub.js create mode 100644 test/fixtures/re-export-star-external.mjs create mode 100644 test/fixtures/sub-directory/re-export-star-external.mjs create mode 100644 test/hook/re-export-star-module.mjs diff --git a/hook.js b/hook.js index 79a2e5b..3c5888b 100644 --- a/hook.js +++ b/hook.js @@ -91,6 +91,10 @@ function isStarExportLine (line) { return /^\* from /.test(line) } +function isBareSpecifier (specifier) { + return /^[a-zA-Z@]/.test(specifier) && !specifier.startsWith('file:') +} + /** * @typedef {object} ProcessedModule * @property {string[]} imports A set of ESM import lines to be added to the @@ -128,6 +132,7 @@ async function processModule ({ srcUrl, context, parentGetSource, + parentResolve, ns = 'namespace', defaultAs = 'default' }) { @@ -154,13 +159,25 @@ async function processModule ({ if (isStarExportLine(n) === true) { const [, modFile] = n.split('* from ') const normalizedModName = normalizeModName(modFile) - const modUrl = new URL(modFile, srcUrl).toString() const modName = Buffer.from(modFile, 'hex') + Date.now() + randomBytes(4).toString('hex') + let modUrl + if (isBareSpecifier(modFile)) { + try { + const result = await parentResolve(modFile, { parentURL: srcUrl }) + modUrl = result.url + } catch (_) { + // ignore + } + } else { + modUrl = new URL(modFile, srcUrl).toString() + } + const data = await processModule({ srcUrl: modUrl, context, parentGetSource, + parentResolve, ns: `$${modName}`, defaultAs: normalizedModName }) @@ -229,7 +246,9 @@ function addIitm (url) { } function createHook (meta) { + let cachedResolve async function resolve (specifier, context, parentResolve) { + cachedResolve = parentResolve const { parentURL = '' } = context const newSpecifier = deleteIitm(specifier) if (isWin && parentURL.indexOf('file:node') === 0) { @@ -269,7 +288,8 @@ function createHook (meta) { const { imports, namespaces, setters: mapSetters } = await processModule({ srcUrl: realUrl, context, - parentGetSource + parentGetSource, + parentResolve: cachedResolve }) const setters = Array.from(mapSetters.values()) diff --git a/test/fixtures/node_modules/some-external-module/index.js b/test/fixtures/node_modules/some-external-module/index.js new file mode 100644 index 0000000..21ec276 --- /dev/null +++ b/test/fixtures/node_modules/some-external-module/index.js @@ -0,0 +1 @@ +export const foo = 'bar' diff --git a/test/fixtures/node_modules/some-external-module/package.json b/test/fixtures/node_modules/some-external-module/package.json new file mode 100644 index 0000000..9e2653d --- /dev/null +++ b/test/fixtures/node_modules/some-external-module/package.json @@ -0,0 +1,8 @@ +{ + "name": "some-external-module", + "type": "module", + "exports": { + ".": "./index.js", + "./sub": "./sub.js" + } +} diff --git a/test/fixtures/node_modules/some-external-module/sub.js b/test/fixtures/node_modules/some-external-module/sub.js new file mode 100644 index 0000000..7e9b246 --- /dev/null +++ b/test/fixtures/node_modules/some-external-module/sub.js @@ -0,0 +1 @@ +export const bar = 'baz' diff --git a/test/fixtures/re-export-star-external.mjs b/test/fixtures/re-export-star-external.mjs new file mode 100644 index 0000000..5d7c2ea --- /dev/null +++ b/test/fixtures/re-export-star-external.mjs @@ -0,0 +1 @@ +export * from 'some-external-module' diff --git a/test/fixtures/sub-directory/re-export-star-external.mjs b/test/fixtures/sub-directory/re-export-star-external.mjs new file mode 100644 index 0000000..7c7d747 --- /dev/null +++ b/test/fixtures/sub-directory/re-export-star-external.mjs @@ -0,0 +1 @@ +export * from 'some-external-module/sub' diff --git a/test/hook/re-export-star-module.mjs b/test/hook/re-export-star-module.mjs new file mode 100644 index 0000000..b796e28 --- /dev/null +++ b/test/hook/re-export-star-module.mjs @@ -0,0 +1,17 @@ +import Hook from '../../index.js' +import { foo } from '../fixtures/re-export-star-external.mjs' +import { bar } from '../fixtures/sub-directory/re-export-star-external.mjs' +import { strictEqual } from 'assert' + +Hook((exports, name) => { + if (name.endsWith('fixtures/re-export-star-external.mjs')) { + exports.foo = '1' + } + + if (name.endsWith('fixtures/sub-directory/re-export-star-external.mjs')) { + exports.bar = '2' + } +}) + +strictEqual(foo, '1') +strictEqual(bar, '2') From 37725a4b834e13b1d1b26168a539d6bc33bdca5c Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 23 May 2024 22:56:56 +0200 Subject: [PATCH 2/5] Don't catch and ignore resolve failures --- hook.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/hook.js b/hook.js index 3c5888b..36b5ed6 100644 --- a/hook.js +++ b/hook.js @@ -163,12 +163,8 @@ async function processModule ({ let modUrl if (isBareSpecifier(modFile)) { - try { const result = await parentResolve(modFile, { parentURL: srcUrl }) modUrl = result.url - } catch (_) { - // ignore - } } else { modUrl = new URL(modFile, srcUrl).toString() } From dfb3949cf11805a83f18714a4693ce675f66ff08 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 24 May 2024 12:05:30 +0200 Subject: [PATCH 3/5] Fix isBareSpecifier --- hook.js | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/hook.js b/hook.js index 36b5ed6..6920ed0 100644 --- a/hook.js +++ b/hook.js @@ -3,6 +3,7 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. const { randomBytes } = require('crypto') +const { URL } = require('url') const specifiers = new Map() const isWin = process.platform === 'win32' @@ -92,7 +93,21 @@ function isStarExportLine (line) { } function isBareSpecifier (specifier) { - return /^[a-zA-Z@]/.test(specifier) && !specifier.startsWith('file:') + // Relative and absolute paths are not bare specifiers. + if ( + specifier.startsWith('.') || + specifier.startsWith('/')) { + return false + } + + // Valid URLs are not bare specifiers. (file:, http:, node:, etc.) + try { + // eslint-disable-next-line no-new + new URL(specifier) + return false + } catch (err) { + return true + } } /** @@ -163,8 +178,9 @@ async function processModule ({ let modUrl if (isBareSpecifier(modFile)) { - const result = await parentResolve(modFile, { parentURL: srcUrl }) - modUrl = result.url + // Bare specifiers need to be resolved relative to the parent module. + const result = await parentResolve(modFile, { parentURL: srcUrl }) + modUrl = result.url } else { modUrl = new URL(modFile, srcUrl).toString() } From 631a3ecafa6c5993048663398cd9db57a04c234f Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 28 May 2024 12:40:10 +0200 Subject: [PATCH 4/5] PR feedback --- hook.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/hook.js b/hook.js index 6920ed0..505a764 100644 --- a/hook.js +++ b/hook.js @@ -101,6 +101,12 @@ function isBareSpecifier (specifier) { } // Valid URLs are not bare specifiers. (file:, http:, node:, etc.) + + // eslint-disable-next-line no-prototype-builtins + if (URL.hasOwnProperty('canParse')) { + return !URL.canParse(specifier) + } + try { // eslint-disable-next-line no-new new URL(specifier) @@ -182,7 +188,7 @@ async function processModule ({ const result = await parentResolve(modFile, { parentURL: srcUrl }) modUrl = result.url } else { - modUrl = new URL(modFile, srcUrl).toString() + modUrl = new URL(modFile, srcUrl).href } const data = await processModule({ From 8e7eb63b7f822e072f7632ec1ed2bf97f0585081 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 28 May 2024 19:17:53 +0200 Subject: [PATCH 5/5] Fix linting --- .../node_modules/some-external-module/{index.js => index.mjs} | 0 test/fixtures/node_modules/some-external-module/package.json | 4 ++-- .../node_modules/some-external-module/{sub.js => sub.mjs} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename test/fixtures/node_modules/some-external-module/{index.js => index.mjs} (100%) rename test/fixtures/node_modules/some-external-module/{sub.js => sub.mjs} (100%) diff --git a/test/fixtures/node_modules/some-external-module/index.js b/test/fixtures/node_modules/some-external-module/index.mjs similarity index 100% rename from test/fixtures/node_modules/some-external-module/index.js rename to test/fixtures/node_modules/some-external-module/index.mjs diff --git a/test/fixtures/node_modules/some-external-module/package.json b/test/fixtures/node_modules/some-external-module/package.json index 9e2653d..2f24602 100644 --- a/test/fixtures/node_modules/some-external-module/package.json +++ b/test/fixtures/node_modules/some-external-module/package.json @@ -2,7 +2,7 @@ "name": "some-external-module", "type": "module", "exports": { - ".": "./index.js", - "./sub": "./sub.js" + ".": "./index.mjs", + "./sub": "./sub.mjs" } } diff --git a/test/fixtures/node_modules/some-external-module/sub.js b/test/fixtures/node_modules/some-external-module/sub.mjs similarity index 100% rename from test/fixtures/node_modules/some-external-module/sub.js rename to test/fixtures/node_modules/some-external-module/sub.mjs