From 647ecefd71f0608cb80ad8bb7068da2c4d85151e Mon Sep 17 00:00:00 2001 From: Ayan Khan Date: Mon, 6 Nov 2023 23:52:30 -0500 Subject: [PATCH] fixes loader cyclical dependency bug by forgoing module wrapping of file and embedding file contents within the patch instead --- hook.js | 46 +++- lib/ast-parse.js | 9 + lib/get-esm-exports.js | 203 +++++++++++++----- package.json | 5 +- test/fixtures/a.mjs | 7 + test/fixtures/b.mjs | 6 + test/fixtures/export-types/declarations.mjs | 11 + .../export-types/default-class-anon.mjs | 1 + test/fixtures/export-types/default-class.mjs | 1 + .../export-types/default-expression-array.mjs | 1 + .../export-types/default-expression-num.mjs | 1 + .../default-expression-string.mjs | 1 + .../export-types/default-function-anon.mjs | 1 + .../export-types/default-function.mjs | 1 + .../export-types/default-generator-anon.mjs | 1 + .../export-types/default-generator.mjs | 1 + test/fixtures/export-types/list.mjs | 12 ++ test/hook/declaration-exports.mjs | 54 +++++ test/hook/default-exports.mjs | 71 ++++++ test/hook/list-exports.mjs | 33 +++ test/other/cyclical-dependency.mjs | 24 +++ 21 files changed, 429 insertions(+), 61 deletions(-) create mode 100644 lib/ast-parse.js create mode 100644 test/fixtures/a.mjs create mode 100644 test/fixtures/b.mjs create mode 100644 test/fixtures/export-types/declarations.mjs create mode 100644 test/fixtures/export-types/default-class-anon.mjs create mode 100644 test/fixtures/export-types/default-class.mjs create mode 100644 test/fixtures/export-types/default-expression-array.mjs create mode 100644 test/fixtures/export-types/default-expression-num.mjs create mode 100644 test/fixtures/export-types/default-expression-string.mjs create mode 100644 test/fixtures/export-types/default-function-anon.mjs create mode 100644 test/fixtures/export-types/default-function.mjs create mode 100644 test/fixtures/export-types/default-generator-anon.mjs create mode 100644 test/fixtures/export-types/default-generator.mjs create mode 100644 test/fixtures/export-types/list.mjs create mode 100644 test/hook/declaration-exports.mjs create mode 100644 test/hook/default-exports.mjs create mode 100644 test/hook/list-exports.mjs create mode 100644 test/other/cyclical-dependency.mjs diff --git a/hook.js b/hook.js index 3639fbf..d5ad87e 100644 --- a/hook.js +++ b/hook.js @@ -2,6 +2,9 @@ // // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. +const fs = require('fs'); +const { fileURLToPath } = require('url') +const astParse = require('./lib/ast-parse.js') const specifiers = new Map() const isWin = process.platform === "win32" @@ -106,9 +109,9 @@ function createHook (meta) { specifiers.set(url.url, specifier) - + return { - url: addIitm(url.url), + url: url.format !== 'module' ? addIitm(url.url) : url.url, shortCircuit: true, format: url.format } @@ -119,7 +122,7 @@ function createHook (meta) { if (hasIitm(url)) { const realUrl = deleteIitm(url) const exportNames = await getExports(realUrl, context, parentGetSource) - return { + return { source: ` import { register } from '${iitmURL}' import * as namespace from ${JSON.stringify(url)} @@ -134,6 +137,39 @@ set.${n} = (v) => { `).join('\n')} register(${JSON.stringify(realUrl)}, namespace, set, ${JSON.stringify(specifiers.get(realUrl))}) ` + } + } + else if (context.format === 'module') { + let fileContents + try { + fileContents = fs.readFileSync(fileURLToPath(url), 'utf8') + } catch (parseError) { + console.error(`Had trouble reading file: ${fileContents}, got error: ${parseError}`); + return parentGetSource(url, context, parentGetSource) + } + try { + const outPut = astParse(fileContents) + fileContents = outPut.code + exportAlias = outPut.exportAlias + } catch (parseError) { + console.error(`Tried AST parsing ${realPath}, got error: ${parseError}`); + return parentGetSource(url, context, parentGetSource) + } + const src = `${fileContents} +import { register } from '${iitmURL}' +const set = {} +const namespace = {} +${Object.entries(exportAlias).map(([key, value]) => ` +set.${key} = (v) => { + ${value} = v; + return true; +}; +namespace.${key} = ${value} +`).join('\n')} +register(${JSON.stringify(url)}, namespace, set, ${JSON.stringify(specifiers.get(url))}) +` + return { + source: src } } @@ -142,7 +178,7 @@ register(${JSON.stringify(realUrl)}, namespace, set, ${JSON.stringify(specifiers // For Node.js 16.12.0 and higher. async function load (url, context, parentLoad) { - if (hasIitm(url)) { + if (hasIitm(url) || context.format === 'module') { const { source } = await getSource(url, context, parentLoad) return { source, @@ -162,7 +198,7 @@ register(${JSON.stringify(realUrl)}, namespace, set, ${JSON.stringify(specifiers resolve, getSource, getFormat (url, context, parentGetFormat) { - if (hasIitm(url)) { + if (hasIitm(url) || context.format === 'module') { return { format: 'module' } diff --git a/lib/ast-parse.js b/lib/ast-parse.js new file mode 100644 index 0000000..0d50761 --- /dev/null +++ b/lib/ast-parse.js @@ -0,0 +1,9 @@ +'use strict' + +const getEsmExports = require('./get-esm-exports.js') + +function astParse(fileContents) { + return getEsmExports(fileContents, true) +} + +module.exports = astParse \ No newline at end of file diff --git a/lib/get-esm-exports.js b/lib/get-esm-exports.js index 3b4fa30..aa928b2 100644 --- a/lib/get-esm-exports.js +++ b/lib/get-esm-exports.js @@ -1,5 +1,6 @@ 'use strict' +const recast = require('recast'); const { Parser } = require('acorn') const { importAssertions } = require('acorn-import-assertions'); @@ -14,82 +15,176 @@ function warn (txt) { process.emitWarning(txt, 'get-esm-exports') } -function getEsmExports (moduleStr) { - const exportedNames = new Set() - const tree = parser.parse(moduleStr, acornOpts) - for (const node of tree.body) { - if (!node.type.startsWith('Export')) continue - switch (node.type) { - case 'ExportNamedDeclaration': - if (node.declaration) { - parseDeclaration(node, exportedNames) - } else { - parseSpecifiers(node, exportedNames) - } - break - case 'ExportDefaultDeclaration': - exportedNames.add('default') - break - case 'ExportAllDeclaration': - if (node.exported) { - exportedNames.add(node.exported.name) - } else { - exportedNames.add('*') - } - break - default: - warn('unrecognized export type: ' + node.type) +function getEsmExports(moduleStr, generate=false) { + const exportSpecifierNames = new Set(); + const exportAlias = {} + const ast = recast.parse(moduleStr, {parser: { + parse(source) { + return parser.parse(source, acornOpts); + } + }}) + + recast.visit(ast, { + visitNode(path) { + const node = path.node; + + if (!node.type.startsWith('Export')) { + this.traverse(path); + return; + } + + switch (node.type) { + case 'ExportNamedDeclaration': + if (node.declaration) { + parseDeclaration(node.declaration, exportAlias); + } else { + parseSpecifiers(node.specifiers, exportAlias, exportSpecifierNames); + } + break; + case 'ExportDefaultDeclaration': + const iitmRenamedExport = 'iitmRenamedExport'; + if (node.declaration.type === 'ObjectExpression' || node.declaration.type === 'ArrayExpression' || node.declaration.type === 'Literal') { + const variableDeclaration = { + type: 'VariableDeclaration', + kind: 'let', + declarations: [ + { + type: 'VariableDeclarator', + id: { + type: 'Identifier', + name: iitmRenamedExport, + }, + init: node.declaration, + }, + ], + }; + path.replace(variableDeclaration); + const newExportDefaultDeclaration = { + type: 'ExportDefaultDeclaration', + declaration: { + type: 'Identifier', + name: iitmRenamedExport, + }, + }; + path.insertAfter(newExportDefaultDeclaration); + } else { + node.declaration.id = { type: 'Identifier', name: iitmRenamedExport }; + } + exportAlias['default'] = iitmRenamedExport; + break; + case 'ExportAllDeclaration': + const exportedName = node.exported ? node.exported.name : '*'; + exportAlias[exportedName] = exportedName; + break; + case 'ExportSpecifier': + exportSpecifierNames.add(node.local.name) + if (node.exported?.name) { + exportAlias[node.exported.name] = node.local.name; + } else if (node.exported?.value) { + exportAlias[node.exported.value] = node.local.name; + } else { + warn('unrecognized specifier export: ' + node.exported); + } + break; + default: + warn('unrecognized export type: ' + node.type); + } + this.traverse(path); + }, + }); + if (exportSpecifierNames.size !== 0) { + convertExportSpecifierToLet(exportSpecifierNames, ast) + } + + if (generate) { + return { + exportAlias: exportAlias, + code: recast.print(ast).code } } - return Array.from(exportedNames) + + return Object.keys(exportAlias) +} + +function convertExportSpecifierToLet(exportSpecifierNames, ast) { + recast.visit(ast, { + visitVariableDeclaration(path) { + const declaration = path.node; + if (declaration.kind === 'const') { + for (const declarator of declaration.declarations) { + const variableName = declarator.id.name; + if (exportSpecifierNames.has(variableName)) { + declaration.kind = 'let'; // Change 'const' to 'let' + } + } + } + this.traverse(path); + }, + }); } -function parseDeclaration (node, exportedNames) { - switch (node.declaration.type) { +function parseDeclaration(declaration, exportAlias) { + switch (declaration.type) { case 'FunctionDeclaration': - exportedNames.add(node.declaration.id.name) - break + exportAlias[declaration.id.name] = declaration.id.name + break; case 'VariableDeclaration': - for (const varDecl of node.declaration.declarations) { - parseVariableDeclaration(varDecl, exportedNames) + for (const varDecl of declaration.declarations) { + if (declaration.kind === 'const') { + declaration.kind = 'let'; + } + parseVariableDeclaration(varDecl, exportAlias); } - break + break; case 'ClassDeclaration': - exportedNames.add(node.declaration.id.name) - break + exportAlias[declaration.id.name] = declaration.id.name + break; default: - warn('unknown declaration type: ' + node.delcaration.type) + warn('unknown declaration type: ' + declaration.type); } } -function parseVariableDeclaration (node, exportedNames) { - switch (node.id.type) { +function parseVariableDeclaration(varDecl, exportAlias) { + switch (varDecl.id.type) { case 'Identifier': - exportedNames.add(node.id.name) - break + exportAlias[varDecl.id.name] = varDecl.id.name + break; case 'ObjectPattern': - for (const prop of node.id.properties) { - exportedNames.add(prop.value.name) + for (const prop of varDecl.id.properties) { + exportAlias[prop.value.name] = prop.value.name } - break + break; case 'ArrayPattern': - for (const elem of node.id.elements) { - exportedNames.add(elem.name) + for (const elem of varDecl.id.elements) { + if (elem) { + exportAlias[elem.name] = elem.name + } } - break + break; default: - warn('unknown variable declaration type: ' + node.id.type) + warn('unknown variable declaration type: ' + varDecl.id.type); } } -function parseSpecifiers (node, exportedNames) { - for (const specifier of node.specifiers) { - if (specifier.exported.type === 'Identifier') { - exportedNames.add(specifier.exported.name) +function parseSpecifiers(specifiers, exportAlias, exportSpecifierNames) { + for (const specifier of specifiers) { + if (specifier.type === 'ExportSpecifier') { + exportSpecifierNames.add(specifier.local.name) + if (specifier.exported?.name) { + exportAlias[specifier.exported.name] = specifier.local.name; + } else if (specifier.exported?.value) { + exportAlias[specifier.exported.value] = specifier.local.name; + } else { + warn('unrecognized specifier export: ' + specifier); + } + } + else if (specifier.exported.type === 'Identifier') { + exportAlias[specifier.exported.name] = specifier.exported.name } else if (specifier.exported.type === 'Literal') { - exportedNames.add(specifier.exported.value) - } else { - warn('unrecognized specifier type: ' + specifier.exported.type) + exportAlias[specifier.exported.value] = specifier.exported.value + } + else { + warn('unrecognized specifier type: ' + specifier.exported.type); } } } diff --git a/package.json b/package.json index 9af84f6..3b24d51 100644 --- a/package.json +++ b/package.json @@ -34,9 +34,10 @@ "typescript": "^4.7.4" }, "dependencies": { - "acorn": "^8.8.2", + "acorn": "^8.11.2", "acorn-import-assertions": "^1.9.0", "cjs-module-lexer": "^1.2.2", - "module-details-from-path": "^1.0.3" + "module-details-from-path": "^1.0.3", + "recast": "^0.23.4" } } diff --git a/test/fixtures/a.mjs b/test/fixtures/a.mjs new file mode 100644 index 0000000..c816a96 --- /dev/null +++ b/test/fixtures/a.mjs @@ -0,0 +1,7 @@ +import { testB } from './b.mjs'; + +export function testA() { + console.log("testA"); +} + +testB(); \ No newline at end of file diff --git a/test/fixtures/b.mjs b/test/fixtures/b.mjs new file mode 100644 index 0000000..72b78e6 --- /dev/null +++ b/test/fixtures/b.mjs @@ -0,0 +1,6 @@ +import { testA } from './a.mjs'; + +export function testB() { + console.log("testB"); + testA(); +} \ No newline at end of file diff --git a/test/fixtures/export-types/declarations.mjs b/test/fixtures/export-types/declarations.mjs new file mode 100644 index 0000000..bc92ca3 --- /dev/null +++ b/test/fixtures/export-types/declarations.mjs @@ -0,0 +1,11 @@ +const o = { name5: 1, name6: 1 }; +const array = [1, 1] + +// Exporting declarations +export let name1 = 1, name2 = 1/*, … */; // also var +export const name3 = 1, name4 = 1/*, … */; // also var, let +export function functionName() { return 1 } +export class ClassName { getFoo() { return 1 } } +export function* generatorFunctionName() { return 1 } +export const { name5, name6: bar } = o; +export const [ name7, name8 ] = array; diff --git a/test/fixtures/export-types/default-class-anon.mjs b/test/fixtures/export-types/default-class-anon.mjs new file mode 100644 index 0000000..41984b3 --- /dev/null +++ b/test/fixtures/export-types/default-class-anon.mjs @@ -0,0 +1 @@ +export default class { getFoo() { return 1 } } diff --git a/test/fixtures/export-types/default-class.mjs b/test/fixtures/export-types/default-class.mjs new file mode 100644 index 0000000..207974b --- /dev/null +++ b/test/fixtures/export-types/default-class.mjs @@ -0,0 +1 @@ +export default class ClassName { getFoo() { return 1 } } diff --git a/test/fixtures/export-types/default-expression-array.mjs b/test/fixtures/export-types/default-expression-array.mjs new file mode 100644 index 0000000..289d4e6 --- /dev/null +++ b/test/fixtures/export-types/default-expression-array.mjs @@ -0,0 +1 @@ +export default [1] \ No newline at end of file diff --git a/test/fixtures/export-types/default-expression-num.mjs b/test/fixtures/export-types/default-expression-num.mjs new file mode 100644 index 0000000..55bb209 --- /dev/null +++ b/test/fixtures/export-types/default-expression-num.mjs @@ -0,0 +1 @@ +export default 1 \ No newline at end of file diff --git a/test/fixtures/export-types/default-expression-string.mjs b/test/fixtures/export-types/default-expression-string.mjs new file mode 100644 index 0000000..a7117c1 --- /dev/null +++ b/test/fixtures/export-types/default-expression-string.mjs @@ -0,0 +1 @@ +export default 'dog' \ No newline at end of file diff --git a/test/fixtures/export-types/default-function-anon.mjs b/test/fixtures/export-types/default-function-anon.mjs new file mode 100644 index 0000000..86d3b3e --- /dev/null +++ b/test/fixtures/export-types/default-function-anon.mjs @@ -0,0 +1 @@ +export default function () { return 1 } \ No newline at end of file diff --git a/test/fixtures/export-types/default-function.mjs b/test/fixtures/export-types/default-function.mjs new file mode 100644 index 0000000..2e15809 --- /dev/null +++ b/test/fixtures/export-types/default-function.mjs @@ -0,0 +1 @@ +export default function functionName() { return 1 } \ No newline at end of file diff --git a/test/fixtures/export-types/default-generator-anon.mjs b/test/fixtures/export-types/default-generator-anon.mjs new file mode 100644 index 0000000..417fbcc --- /dev/null +++ b/test/fixtures/export-types/default-generator-anon.mjs @@ -0,0 +1 @@ +export default function* () { return 1 } \ No newline at end of file diff --git a/test/fixtures/export-types/default-generator.mjs b/test/fixtures/export-types/default-generator.mjs new file mode 100644 index 0000000..1a4f33b --- /dev/null +++ b/test/fixtures/export-types/default-generator.mjs @@ -0,0 +1 @@ +export default function* generatorFunctionName() { return 1 } \ No newline at end of file diff --git a/test/fixtures/export-types/list.mjs b/test/fixtures/export-types/list.mjs new file mode 100644 index 0000000..512a87c --- /dev/null +++ b/test/fixtures/export-types/list.mjs @@ -0,0 +1,12 @@ +// Export list +const name1 = 1 +const name2 = 1 +const name5 = 1 +const variable1 = 1 +const variable2 = 1 +const variable3 = 1 +const name6 = 1 +export { name1, name2 }; +export { variable1 as name3, variable2 as name4, /* …, */ name5 }; +export { variable3 as "name" }; +export { name6 as default /*, … */ }; diff --git a/test/hook/declaration-exports.mjs b/test/hook/declaration-exports.mjs new file mode 100644 index 0000000..734328e --- /dev/null +++ b/test/hook/declaration-exports.mjs @@ -0,0 +1,54 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2.0 License. +// +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. + +import Hook from '../../index.js' +import { name1 as n1, + name2 as n2, + name3 as n3, + name4 as n4, + functionName as fn, + ClassName as cn, + generatorFunctionName as gfn, + name5 as n5, + bar as n6, + name7 as n7, + name8 as n8 + } from '../fixtures/export-types/declarations.mjs' +import { strictEqual } from 'assert' + +Hook((exports, name) => { + if (name.match(/declarations\.m?js/)) { + exports.name1 += 1 + exports.name2 += 1 + exports.name3 += 1 + exports.name4 += 1 + const orig = exports.functionName + exports.functionName = function () { + return orig() + 1 + } + exports.ClassName.prototype.getFoo = function () { + return 2 + } + const orig2 = exports.generatorFunctionName + exports.generatorFunctionName = function* () { + return orig2().next().value + 1 + } + exports.name5 += 1 + exports.bar += 1 + exports.name7 += 1 + exports.name8 += 1 + } +}) + +strictEqual(n1, 2) +strictEqual(n2, 2) +strictEqual(n3, 2) +strictEqual(n4, 2) +strictEqual(fn(), 2) +strictEqual(new cn().getFoo(), 2) +strictEqual(gfn().next().value, 2) +strictEqual(n5, 2) +strictEqual(n6, 2) +strictEqual(n7, 2) +strictEqual(n8, 2) diff --git a/test/hook/default-exports.mjs b/test/hook/default-exports.mjs new file mode 100644 index 0000000..fd417fb --- /dev/null +++ b/test/hook/default-exports.mjs @@ -0,0 +1,71 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2.0 License. +// +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. + +import Hook from '../../index.js' +import a from '../fixtures/export-types/default-expression-array.mjs' +import n from '../fixtures/export-types/default-expression-num.mjs' +import s from '../fixtures/export-types/default-expression-string.mjs' +import fn from '../fixtures/export-types/default-function.mjs' +import cn from '../fixtures/export-types/default-class.mjs' +import gfn from '../fixtures/export-types/default-generator.mjs' +import afn from '../fixtures/export-types/default-function-anon.mjs' +import acn from '../fixtures/export-types/default-class-anon.mjs' +import agfn from '../fixtures/export-types/default-generator-anon.mjs' +import { strictEqual } from 'assert' + +Hook((exports, name) => { + if (name.match(/default-expression-array\.m?js/)) { + exports.default[0] += 1 + } else if (name.match(/default-expression-num\.m?js/)) { + exports.default += 1 + } + else if (name.match(/default-expression-string\.m?js/)) { + exports.default += 'dawg' + } + else if (name.match(/default-function\.m?js/)) { + const orig = exports.default + exports.default = function () { + return orig() + 1 + } + } + else if (name.match(/default-class\.m?js/)) { + exports.default.prototype.getFoo = function () { + return 2 + } + } + else if (name.match(/default-generator\.m?js/)) { + const orig2 = exports.default + exports.default = function* () { + return orig2().next().value + 1 + } + } + else if (name.match(/default-function-anon\.m?js/)) { + const orig = exports.default + exports.default = function () { + return orig() + 1 + } + } + else if (name.match(/default-class-anon\.m?js/)) { + exports.default.prototype.getFoo = function () { + return 2 + } + } + else if (name.match(/default-generator-anon\.m?js/)) { + const orig2 = exports.default + exports.default = function* () { + return orig2().next().value + 1 + } + } +}) + +strictEqual(a[0], 2) +strictEqual(fn(), 2) +strictEqual(new cn().getFoo(), 2) +strictEqual(gfn().next().value, 2) +strictEqual(afn(), 2) +strictEqual(new acn().getFoo(), 2) +strictEqual(agfn().next().value, 2) +// the below tests won't work because literal default exports are static +// strictEqual(n, 2) +// strictEqual(s, 'dogdawg') diff --git a/test/hook/list-exports.mjs b/test/hook/list-exports.mjs new file mode 100644 index 0000000..f181471 --- /dev/null +++ b/test/hook/list-exports.mjs @@ -0,0 +1,33 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2.0 License. +// +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. + +import Hook from '../../index.js' +import n, { name1 as n1, + name2 as n2, + name3 as n3, + name4 as n4, + name5 as n5, + "name" as n6, + } from '../fixtures/export-types/list.mjs' +import { strictEqual } from 'assert' + +Hook((exports, name) => { + if (name.match(/list\.m?js/)) { + exports.name1 += 1 + exports.name2 += 1 + exports.name3 += 1 + exports.name4 += 1 + exports.name5 += 1 + exports.name += 1 + exports.default += 1 + } +}) + +strictEqual(n1, 2) +strictEqual(n2, 2) +strictEqual(n3, 2) +strictEqual(n4, 2) +strictEqual(n5, 2) +strictEqual(n6, 2) +strictEqual(n, 2) diff --git a/test/other/cyclical-dependency.mjs b/test/other/cyclical-dependency.mjs new file mode 100644 index 0000000..ebb163b --- /dev/null +++ b/test/other/cyclical-dependency.mjs @@ -0,0 +1,24 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2.0 License. +// +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc. + +import { spawn } from 'child_process'; +import { strictEqual } from 'assert'; + +const nodeProcess = spawn('node', ['--loader', './hook.mjs', './test/fixtures/a.mjs']); +const expectedOutput = 'testB\ntestA' +let stdout = ''; +let stderr = ''; + +nodeProcess.stdout.on('data', (data) => { + stdout += data.toString(); +}); + +nodeProcess.stderr.on('data', (data) => { + stderr += data.toString(); +}); + +nodeProcess.on('close', (code) => { + strictEqual(stderr, '', 'There should be no errors on stderr'); + strictEqual(stdout.trim(), expectedOutput, 'The stdout should match the expected output'); +});