From cbe0eb1337c142427e08079cede1f1b7ba632b94 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 23 May 2024 23:15:30 +0200 Subject: [PATCH 1/4] Fix duplicate declarations --- hook.js | 4 ++-- test/fixtures/duplicate-a.mjs | 1 + test/fixtures/duplicate-b.mjs | 1 + test/fixtures/duplicate.mjs | 2 ++ test/hook/duplicate-exports.mjs | 11 +++++++++++ 5 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/duplicate-a.mjs create mode 100644 test/fixtures/duplicate-b.mjs create mode 100644 test/fixtures/duplicate.mjs create mode 100644 test/hook/duplicate-exports.mjs diff --git a/hook.js b/hook.js index 79a2e5b..0005125 100644 --- a/hook.js +++ b/hook.js @@ -180,7 +180,7 @@ async function processModule ({ // needs to utilize that new name while being initialized from the // corresponding origin namespace. const renamedExport = matches[2] - setters.set(`$${renamedExport}${ns}`, ` + setters.set(`$${renamedExport}`, ` let $${renamedExport} = ${ns}.default export { $${renamedExport} as ${renamedExport} } set.${renamedExport} = (v) => { @@ -191,7 +191,7 @@ async function processModule ({ continue } - setters.set(`$${n}` + ns, ` + setters.set(`$${n}`, ` let $${n} = ${ns}.${n} export { $${n} as ${n} } set.${n} = (v) => { diff --git a/test/fixtures/duplicate-a.mjs b/test/fixtures/duplicate-a.mjs new file mode 100644 index 0000000..a41341d --- /dev/null +++ b/test/fixtures/duplicate-a.mjs @@ -0,0 +1 @@ +export const foo = 'a' diff --git a/test/fixtures/duplicate-b.mjs b/test/fixtures/duplicate-b.mjs new file mode 100644 index 0000000..853627d --- /dev/null +++ b/test/fixtures/duplicate-b.mjs @@ -0,0 +1 @@ +export const foo = 'b' diff --git a/test/fixtures/duplicate.mjs b/test/fixtures/duplicate.mjs new file mode 100644 index 0000000..adef26f --- /dev/null +++ b/test/fixtures/duplicate.mjs @@ -0,0 +1,2 @@ +export * from './duplicate-a.mjs' +export * from './duplicate-b.mjs' diff --git a/test/hook/duplicate-exports.mjs b/test/hook/duplicate-exports.mjs new file mode 100644 index 0000000..f85f3d0 --- /dev/null +++ b/test/hook/duplicate-exports.mjs @@ -0,0 +1,11 @@ +import Hook from '../../index.js' +import { foo } from '../fixtures/duplicate.mjs' +import { strictEqual } from 'assert' + +Hook((exports, name) => { + if (name.endsWith('/duplicate.mjs')) { + exports.foo = '1' + } +}) + +strictEqual(foo, '1') From 52c5ac5a8d20c51a99d8843f759468dd5862e7c4 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 23 May 2024 23:19:05 +0200 Subject: [PATCH 2/4] Test for which export takes priority --- test/hook/duplicate-exports.mjs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/hook/duplicate-exports.mjs b/test/hook/duplicate-exports.mjs index f85f3d0..296af64 100644 --- a/test/hook/duplicate-exports.mjs +++ b/test/hook/duplicate-exports.mjs @@ -4,6 +4,8 @@ import { strictEqual } from 'assert' Hook((exports, name) => { if (name.endsWith('/duplicate.mjs')) { + // The last export always takes priority + strictEqual(exports.foo, 'b') exports.foo = '1' } }) From b0e4e4cbe22bf6cd548f02d71ed1bb2c2c997def Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 28 May 2024 19:26:48 +0200 Subject: [PATCH 3/4] Simplify test since the exports vary by node version --- test/hook/duplicate-exports.mjs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/test/hook/duplicate-exports.mjs b/test/hook/duplicate-exports.mjs index 296af64..2523180 100644 --- a/test/hook/duplicate-exports.mjs +++ b/test/hook/duplicate-exports.mjs @@ -1,13 +1,4 @@ -import Hook from '../../index.js' -import { foo } from '../fixtures/duplicate.mjs' -import { strictEqual } from 'assert' +import * as lib from '../fixtures/duplicate.mjs' +import { notEqual } from 'assert' -Hook((exports, name) => { - if (name.endsWith('/duplicate.mjs')) { - // The last export always takes priority - strictEqual(exports.foo, 'b') - exports.foo = '1' - } -}) - -strictEqual(foo, '1') +notEqual(lib, undefined) From fcda35a3c8decf28df46fa540839183de8692039 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 29 May 2024 02:54:28 +0200 Subject: [PATCH 4/4] Exclude duplicate exports --- hook.js | 27 +++++++++++++++++++++++---- test/hook/duplicate-exports.mjs | 4 +++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/hook.js b/hook.js index 594689b..f333731 100644 --- a/hook.js +++ b/hook.js @@ -116,6 +116,25 @@ function isBareSpecifier (specifier) { } } +function mapExcludingDuplicates () { + const map = new Map() + const duplicates = new Set() + return { + set (k, v, isDefault = false) { + if (map.has(k)) { + if (!isDefault) { + duplicates.add(k) + map.delete(k) + } + } else { + map.set(k, v) + } + }, + values: () => map.values(), + entries: () => map.entries() + } +} + /** * @typedef {object} ProcessedModule * @property {string[]} imports A set of ESM import lines to be added to the @@ -174,7 +193,7 @@ async function processModule ({ // 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() + const setters = mapExcludingDuplicates() for (const n of exportNames) { if (isStarExportLine(n) === true) { @@ -215,18 +234,18 @@ async function processModule ({ // needs to utilize that new name while being initialized from the // corresponding origin namespace. const renamedExport = matches[2] - setters.set(`$${renamedExport}`, ` + setters.set(renamedExport, ` let $${renamedExport} = ${ns}.default export { $${renamedExport} as ${renamedExport} } set.${renamedExport} = (v) => { $${renamedExport} = v return true } - `) + `, true) continue } - setters.set(`$${n}`, ` + setters.set(n, ` let $${n} = ${ns}.${n} export { $${n} as ${n} } set.${n} = (v) => { diff --git a/test/hook/duplicate-exports.mjs b/test/hook/duplicate-exports.mjs index 2523180..56d012d 100644 --- a/test/hook/duplicate-exports.mjs +++ b/test/hook/duplicate-exports.mjs @@ -1,4 +1,6 @@ import * as lib from '../fixtures/duplicate.mjs' -import { notEqual } from 'assert' +import { notEqual, strictEqual } from 'assert' notEqual(lib, undefined) +// foo should not be exported because there are duplicate exports +strictEqual(lib.foo, undefined)