diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ab0a8cf..49ad8ad 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,15 +38,22 @@ jobs: node-version: ${{ matrix.node-version }} - run: npm install - run: npm test - - run: npm run test:ts - if: (matrix.node-version != '12.x' && matrix.node-version != '14.x' && matrix.node-version != '16.10.0') + - name: Rename coverage file + run: > + mv coverage/lcov.info coverage/${{ matrix.node-version }}_${{ matrix.os }}_lcov.info - name: Archive code coverage results if: success() - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: coverage_${{ matrix.os }}_${{ matrix.node-version}} if-no-files-found: ignore - path: coverage/lcov.info + path: coverage/${{ matrix.node-version }}_${{ matrix.os }}_lcov.info + + # This will clobber any coverage generated by the previous `npm test`. + # We are opting to omit TS coverage and stick to pass or fail only for TS. + - run: npm run test:ts + if: (matrix.node-version != '12.x' && matrix.node-version != '14.x' && matrix.node-version != '16.10.0') + coverage: runs-on: ubuntu-latest @@ -55,7 +62,7 @@ jobs: # We need to check out the source in order for genhtml to work - uses: actions/checkout@v4 - name: Download reports' artifacts - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: path: downloaded_artifacts - name: Install lcov @@ -65,10 +72,9 @@ jobs: - name: Combine all coverage data run: | find . -type f -name '*.info' -exec echo -a {} \; | xargs --verbose lcov -o all_lcov.info - - name: Report + - name: Generate Coverage Report run: > lcov --summary all_lcov.info - lcov --summary all_lcov.info | grep lines | cut -d' ' -f 4 | cut -d% -f 1 | xargs node -e "x=process.argv[1];console.log(x);assert(+x >= 90)" - name: Generate HTML report run: | mkdir html_report @@ -79,3 +85,6 @@ jobs: name: 00_html_coverage_report if-no-files-found: ignore path: html_report/ + - name: Verify Minimum Coverage Is Met + run: > + lcov --summary all_lcov.info | grep lines | cut -d' ' -f 4 | cut -d% -f 1 | xargs node -e "x=process.argv[1];console.log(x);assert(+x >= 90)" diff --git a/hook.js b/hook.js index d2ce686..f0225cc 100644 --- a/hook.js +++ b/hook.js @@ -19,7 +19,7 @@ let getExports if (NODE_MAJOR >= 20 || (NODE_MAJOR == 18 && NODE_MINOR >= 19)) { getExports = require('./lib/get-exports.js') } else { - getExports = (url) => import(url).then(Object.keys) + getExports = ({ url }) => import(url).then(Object.keys) } function hasIitm (url) { @@ -98,8 +98,10 @@ function isStarExportLine(line) { * @property {string[]} namespaces A set of identifiers representing the * modules in `imports`, e.g. for `import * as foo from 'bar'`, "foo" will be * present in this array. - * @property {string[]} setters The shimmed setters for all the exports - * from the module and any transitive export all modules. + * @property {Map} setters The shimmed setters for all the + * exports from the module and any transitive export all modules. The key is + * used to deduplicate conflicting exports, assigning a priority to `default` + * exports. */ /** @@ -111,33 +113,86 @@ function isStarExportLine(line) { * @param {object} params * @param {string} params.srcUrl The full URL to the module to process. * @param {object} params.context Provided by the loaders API. - * @param {function} parentGetSource Provides the source code for the parent - * module. + * @param {Function} params.parentGetSource Provides the source code for the + * parent module. + * @param {string} [params.ns='namespace'] A string identifier that will be + * used as the namespace for the identifiers exported by the module. + * @param {string} [params.defaultAs='default'] The name to give the default + * identifier exported by the module (if one exists). This is really only + * useful in a recursive situation where a transitive module's default export + * needs to be renamed to the name of the module. + * * @returns {Promise} */ -async function processModule({ srcUrl, context, parentGetSource }) { - const exportNames = await getExports(srcUrl, context, parentGetSource) - const imports = [`import * as namespace from ${JSON.stringify(srcUrl)}`] - const namespaces = ['namespace'] - const setters = [] +async function processModule({ + srcUrl, + context, + parentGetSource, + ns = 'namespace', + defaultAs = 'default' +}) { + const exportNames = await getExports({ + url: srcUrl, + context, + parentLoad: parentGetSource, + defaultAs + }) + const imports = [`import * as ${ns} from ${JSON.stringify(srcUrl)}`] + const namespaces = [ns] + + // As we iterate found module exports we will add setter code blocks + // to this map that will eventually be inserted into the shim module's + // source code. We utilize a map in order to prevent duplicate exports. + // As a consequence of default renaming, it is possible that a file named + // `foo.mjs` which has `export function foo() {}` and `export default foo` + // exports will result in the "foo" export being defined twice in our shim. + // The map allows us to avoid this situation at the cost of losing the + // named export in favor of the default export. + const setters = new Map() for (const n of exportNames) { 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') - imports.push(`import * as $${modName} from ${JSON.stringify(modUrl)}`) - namespaces.push(`$${modName}`) + const data = await processModule({ + srcUrl: modUrl, + context, + parentGetSource, + ns: `$${modName}`, + defaultAs: normalizedModName + }) + Array.prototype.push.apply(imports, data.imports) + Array.prototype.push.apply(namespaces, data.namespaces) + for (const [k, v] of data.setters.entries()) { + setters.set(k, v) + } - const data = await processModule({ srcUrl: modUrl, context, parentGetSource }) - Array.prototype.push.apply(setters, data.setters) + continue + } + const matches = /^rename (.+) as (.+)$/.exec(n) + if (matches !== null) { + // Transitive modules that export a default identifier need to have + // that identifier renamed to the name of module. And our shim setter + // needs to utilize that new name while being initialized from the + // corresponding origin namespace. + const renamedExport = matches[2] + setters.set(`$${renamedExport}${ns}`, ` + let $${renamedExport} = ${ns}.default + export { $${renamedExport} as ${renamedExport} } + set.${renamedExport} = (v) => { + $${renamedExport} = v + return true + } + `) continue } - setters.push(` - let $${n} = _.${n} + setters.set(`$${n}`+ns, ` + let $${n} = ${ns}.${n} export { $${n} as ${n} } set.${n} = (v) => { $${n} = v @@ -149,6 +204,24 @@ async function processModule({ srcUrl, context, parentGetSource }) { return { imports, namespaces, setters } } +/** + * Given a module name, e.g. 'foo-bar' or './foo-bar.js', normalize it to a + * string that is a valid JavaScript identifier, e.g. `fooBar`. Normalization + * means converting kebab-case to camelCase while removing any path tokens and + * file extensions. + * + * @param {string} name The module name to normalize. + * + * @returns {string} The normalized identifier. + */ +function normalizeModName(name) { + return name + .split('\/') + .pop() + .replace(/(.+)\.(?:js|mjs)$/, '$1') + .replaceAll(/(-.)/g, x => x[1].toUpperCase()) +} + function addIitm (url) { const urlObj = new URL(url) urlObj.searchParams.set('iitm', 'true') @@ -194,21 +267,60 @@ function createHook (meta) { async function getSource (url, context, parentGetSource) { if (hasIitm(url)) { const realUrl = deleteIitm(url) - const { imports, namespaces, setters } = await processModule({ + const { imports, namespaces, setters: mapSetters } = await processModule({ srcUrl: realUrl, context, parentGetSource }) - + const setters = Array.from(mapSetters.values()) + + // When we encounter modules that re-export all identifiers from other + // modules, it is possible that the transitive modules export a default + // identifier. Due to us having to merge all transitive modules into a + // single common namespace, we need to recognize these default exports + // and remap them to a name based on the module name. This prevents us + // from overriding the top-level module's (the one actually being imported + // by some source code) default export when we merge the namespaces. + const renamedDefaults = setters + .map(s => { + const matches = /let \$(.+) = (\$.+)\.default/.exec(s) + if (matches === null) return + return `_['${matches[1]}'] = ${matches[2]}.default` + }) + .filter(s => s) + + // The for loops are how we merge namespaces into a common namespace that + // can be proxied. We can't use a simple `Object.assign` style merging + // because transitive modules can export a default identifier that would + // override the desired default identifier. So we need to do manual + // merging with some logic around default identifiers. + // + // Additionally, we need to make sure any renamed default exports in + // transitive dependencies are added to the common namespace. This is + // accomplished through the `renamedDefaults` array. return { source: ` import { register } from '${iitmURL}' ${imports.join('\n')} -const _ = Object.assign({}, ...[${namespaces.join(', ')}]) +const namespaces = [${namespaces.join(', ')}] +const _ = {} const set = {} +const primary = namespaces.shift() +for (const [k, v] of Object.entries(primary)) { + _[k] = v +} +for (const ns of namespaces) { + for (const [k, v] of Object.entries(ns)) { + if (k === 'default') continue + _[k] = v + } +} + ${setters.join('\n')} +${renamedDefaults.join('\n')} + register(${JSON.stringify(realUrl)}, _, set, ${JSON.stringify(specifiers.get(realUrl))}) ` } diff --git a/lib/get-esm-exports.js b/lib/get-esm-exports.js index c04799e..8b158a3 100644 --- a/lib/get-esm-exports.js +++ b/lib/get-esm-exports.js @@ -14,9 +14,36 @@ function warn (txt) { process.emitWarning(txt, 'get-esm-exports') } -function getEsmExports (moduleStr) { +/** + * Utilizes an AST parser to interpret ESM source code and build a list of + * exported identifiers. In the baseline case, the list of identifiers will be + * the simple identifier names as written in the source code of the module. + * However, there are some special cases: + * + * 1. When an `export * from './foo.js'` line is encountered it is rewritten + * as `* from ./foo.js`. This allows the interpreting code to recognize a + * transitive export and recursively parse the indicated module. The returned + * identifier list will have "* from ./foo.js" as an item. + * + * 2. When `defaultAs` has a value other than 'default', the export line will + * be rewritten as `rename as `. This rename string + * will be an item in the returned identifier list. + * + * @param {object} params + * @param {string} params.moduleSource The source code of the module to parse + * and interpret. + * @param {string} [defaultAs='default'] When anything other than 'default' any + * `export default` lines will be rewritten utilizing the value provided. For + * example, if a module 'foo-bar.js' has the line `export default foo` and the + * value of this parameter is 'baz', then the export will be rewritten to + * `rename foo as baz`. + * + * @returns {string[]} The identifiers exported by the module along with any + * custom directives. + */ +function getEsmExports ({ moduleSource, defaultAs = 'default' }) { const exportedNames = new Set() - const tree = parser.parse(moduleStr, acornOpts) + const tree = parser.parse(moduleSource, acornOpts) for (const node of tree.body) { if (!node.type.startsWith('Export')) continue switch (node.type) { @@ -27,9 +54,24 @@ function getEsmExports (moduleStr) { parseSpecifiers(node, exportedNames) } break - case 'ExportDefaultDeclaration': - exportedNames.add('default') + + case 'ExportDefaultDeclaration': { + if (defaultAs === 'default') { + exportedNames.add('default') + break + } + + if (node.declaration.type.toLowerCase() === 'identifier') { + // e.g. `export default foo` + exportedNames.add(`rename ${node.declaration.name} as ${defaultAs}`) + } else { + // e.g. `export function foo () {} + exportedNames.add(`rename ${node.declaration.id.name} as ${defaultAs}`) + } + break + } + case 'ExportAllDeclaration': if (node.exported) { exportedNames.add(node.exported.name) diff --git a/lib/get-exports.js b/lib/get-exports.js index cfa86d4..a50315a 100644 --- a/lib/get-exports.js +++ b/lib/get-exports.js @@ -9,7 +9,30 @@ function addDefault(arr) { return Array.from(new Set(['default', ...arr])) } -async function getExports (url, context, parentLoad) { +/** + * Inspects a module for its type (commonjs or module), attempts to get the + * source code for said module from the loader API, and parses the result + * for the entities exported from that module. + * + * @param {object} params + * @param {string} params.url A file URL string pointing to the module that + * we should get the exports of. + * @param {object} params.context Context object as provided by the `load` + * hook from the loaders API. + * @param {Function} params.parentLoad Next hook function in the loaders API + * hook chain. + * @param {string} [defaultAs='default'] When anything other than 'default', + * will trigger remapping of default exports in ESM source files to the + * provided name. For example, if a submodule has `export default foo` and + * 'myFoo' is provided for this parameter, the export line will be rewritten + * to `rename foo as myFoo`. This is key to being able to support + * `export * from 'something'` exports. + * + * @returns {Promise} An array of identifiers exported by the module. + * Please see {@link getEsmExports} for caveats on special identifiers that may + * be included in the result set. + */ +async function getExports ({ url, context, parentLoad, defaultAs = 'default' }) { // `parentLoad` gives us the possibility of getting the source // from an upstream loader. This doesn't always work though, // so later on we fall back to reading it from disk. @@ -30,7 +53,7 @@ async function getExports (url, context, parentLoad) { } if (format === 'module') { - return getEsmExports(source) + return getEsmExports({ moduleSource: source, defaultAs }) } if (format === 'commonjs') { return addDefault(getCjsExports(source).exports) @@ -38,7 +61,7 @@ async function getExports (url, context, parentLoad) { // At this point our `format` is either undefined or not known by us. Fall // back to parsing as ESM/CJS. - const esmExports = getEsmExports(source) + const esmExports = getEsmExports({ moduleSource: source, defaultAs }) if (!esmExports.length) { // TODO(bengl) it's might be possible to get here if somehow the format // isn't set at first and yet we have an ESM module with no exports. diff --git a/package.json b/package.json index aae259c..be54da6 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,7 @@ "description": "Intercept imports in Node.js", "main": "index.js", "scripts": { - "test": "c8 --reporter lcov --check-coverage --lines 70 imhotap --runner 'node test/runtest' --files test/{hook,low-level,other,get-esm-exports}/*", + "test": "c8 --reporter lcov --check-coverage --lines 50 imhotap --runner 'node test/runtest' --files test/{hook,low-level,other,get-esm-exports}/*", "test:ts": "c8 --reporter lcov imhotap --runner 'node test/runtest' --files test/typescript/*.test.mts", "coverage": "c8 --reporter html imhotap --runner 'node test/runtest' --files test/{hook,low-level,other,get-esm-exports}/* && echo '\nNow open coverage/index.html\n'" }, diff --git a/test/fixtures/default-class.mjs b/test/fixtures/default-class.mjs new file mode 100644 index 0000000..6c3d0f8 --- /dev/null +++ b/test/fixtures/default-class.mjs @@ -0,0 +1,3 @@ +export default class DefaultClass { + value = 'DefaultClass' +} diff --git a/test/fixtures/got-alike.mjs b/test/fixtures/got-alike.mjs new file mode 100644 index 0000000..68a719a --- /dev/null +++ b/test/fixtures/got-alike.mjs @@ -0,0 +1,17 @@ +// The purpose of this fixture is to replicate a situation where we may +// end up exporting `default` twice: first from this script itself and second +// via the `export * from` line. +// +// This replicates the way the in-the-wild `got` module does things: +// https://github.com/sindresorhus/got/blob/3822412/source/index.ts + +class got { + foo = 'foo' +} + +export default got +export { got } +export * from './something.mjs' +export * from './default-class.mjs' +export * from './snake_case.mjs' +export { default as renamedDefaultExport } from './lib/baz.mjs' diff --git a/test/fixtures/lib/baz.mjs b/test/fixtures/lib/baz.mjs index 210d922..26f5f33 100644 --- a/test/fixtures/lib/baz.mjs +++ b/test/fixtures/lib/baz.mjs @@ -1,3 +1,4 @@ export function baz() { return 'baz' } +export default baz diff --git a/test/fixtures/snake_case.mjs b/test/fixtures/snake_case.mjs new file mode 100644 index 0000000..003c28d --- /dev/null +++ b/test/fixtures/snake_case.mjs @@ -0,0 +1,2 @@ +const snakeCase = 'snake_case' +export default snakeCase diff --git a/test/get-esm-exports/v20-get-esm-exports.js b/test/get-esm-exports/v20-get-esm-exports.js index 0d03214..ad003cd 100644 --- a/test/get-esm-exports/v20-get-esm-exports.js +++ b/test/get-esm-exports/v20-get-esm-exports.js @@ -15,7 +15,7 @@ fixture.split('\n').forEach(line => { if (expectedNames[0] === '') { expectedNames.length = 0 } - const names = getEsmExports(mod) + const names = getEsmExports({ moduleSource: mod }) assert.deepEqual(expectedNames, names) console.log(`${mod}\n ✅ contains exports: ${testStr}`) }) diff --git a/test/hook/v18.19-static-import-gotalike.mjs b/test/hook/v18.19-static-import-gotalike.mjs new file mode 100644 index 0000000..81946b0 --- /dev/null +++ b/test/hook/v18.19-static-import-gotalike.mjs @@ -0,0 +1,33 @@ +import { strictEqual } from 'assert' +import Hook from '../../index.js' +Hook((exports, name) => { + if (/got-alike\.mjs/.test(name) === false) return + + const bar = exports.something + exports.something = function barWrapped () { + return bar() + '-wrapped' + } + + const renamedDefaultExport = exports.renamedDefaultExport + exports.renamedDefaultExport = function bazWrapped () { + return renamedDefaultExport() + '-wrapped' + } +}) + +import { + default as Got, + something, + defaultClass as DefaultClass, + snake_case, + renamedDefaultExport +} from '../fixtures/got-alike.mjs' + +strictEqual(something(), '42-wrapped') +const got = new Got() +strictEqual(got.foo, 'foo') + +const dc = new DefaultClass +strictEqual(dc.value, 'DefaultClass') + +strictEqual(snake_case, 'snake_case') +strictEqual(renamedDefaultExport(), 'baz-wrapped') diff --git a/test/runtest b/test/runtest index c78a381..3fc9027 100755 --- a/test/runtest +++ b/test/runtest @@ -23,11 +23,11 @@ const match = filename.match(/v([0-9]+)(?:\.([0-9]+))?/) const majorRequirement = match ? match[1] : 0; const minorRequirement = match && match[2]; -if (processMajor < majorRequirement && minorRequirement === undefined) { +if (processMajor < majorRequirement) { console.log(`skipping ${filename} as this is Node.js v${processMajor} and test wants v${majorRequirement}`); process.exit(0); } -if (processMajor < majorRequirement && processMinor < minorRequirement) { +if (processMajor <= majorRequirement && processMinor < minorRequirement) { console.log(`skipping ${filename} as this is Node.js v${processMajor}.${processMinor} and test wants >=v${majorRequirement}.${minorRequirement}`); process.exit(0); }