Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Incorrect exports #87

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 24 additions & 48 deletions hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,24 @@ function isBareSpecifier (specifier) {
}
}

function mapExcludingDuplicates () {
const map = new Map()
const duplicates = new Set()
return {
set (k, v) {
if (map.has(k)) {
duplicates.add(k)
map.delete(k)
} else if (!duplicates.has(k)) {
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
Expand Down Expand Up @@ -169,12 +187,7 @@ async function processModule ({
// 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()
const setters = mapExcludingDuplicates()

for (const n of exportNames) {
if (isStarExportLine(n) === true) {
Expand Down Expand Up @@ -208,25 +221,7 @@ async function processModule ({
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.set(`$${n}` + ns, `
setters.set(n, `
let $${n} = ${ns}.${n}
export { $${n} as ${n} }
set.${n} = (v) => {
Expand Down Expand Up @@ -320,30 +315,11 @@ function createHook (meta) {
})
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 undefined
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}'
Expand All @@ -358,16 +334,16 @@ const primary = namespaces.shift()
for (const [k, v] of Object.entries(primary)) {
_[k] = v
}

${setters.join('\n')}

for (const ns of namespaces) {
for (const [k, v] of Object.entries(ns)) {
if (k === 'default') continue
_[k] = v
if (k in set) _[k] = v
}
}

${setters.join('\n')}
${renamedDefaults.join('\n')}

register(${JSON.stringify(realUrl)}, _, set, ${JSON.stringify(specifiers.get(realUrl))})
`
}
Expand Down
6 changes: 3 additions & 3 deletions lib/get-esm-exports.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ function getEsmExports ({ moduleSource, defaultAs = 'default' }) {

if (node.declaration.type.toLowerCase() === 'identifier') {
// e.g. `export default foo`
exportedNames.add(`rename ${node.declaration.name} as ${defaultAs}`)
} else {
exportedNames.add(node.declaration.name)
} else if (node.declaration && node.declaration.id && node.declaration.id.name) {
// e.g. `export function foo () {}
exportedNames.add(`rename ${node.declaration.id.name} as ${defaultAs}`)
exportedNames.add(node.declaration.id.name)
}

break
Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/duplicate-a.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export const foo = 'a'
export default function () {
return 'c'
}
1 change: 1 addition & 0 deletions test/fixtures/duplicate-b.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const foo = 'a'
5 changes: 5 additions & 0 deletions test/fixtures/duplicate.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export * from './duplicate-a.mjs'
export * from './duplicate-b.mjs'

const foo = 'foo'
export default foo
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './default-call-expression.mjs'
export { default as somethingElse } from './default-call-expression.mjs'
1 change: 1 addition & 0 deletions test/fixtures/export-types/default-call-expression.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default parseInt('1')
8 changes: 8 additions & 0 deletions test/hook/default-export.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ 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 callEx from '../fixtures/export-types/default-call-expression.mjs'
import { somethingElse } from '../fixtures/export-types/default-call-expression-renamed.mjs'
import defaultImportExport from '../fixtures/export-types/import-default-export.mjs'
import varDefaultExport from '../fixtures/export-types/variable-default-export.mjs'
import { strictEqual } from 'assert'
Expand Down Expand Up @@ -61,6 +63,10 @@ Hook((exports, name) => {
exports.default = function () {
return orig4() + 1
}
} else if (name.match(/default-call-expression\.m?js/)) {
exports.default += 1
} else if (name.match(/default-call-expression-renamed\.m?js/)) {
exports.somethingElse += 1
}
})

Expand All @@ -76,3 +82,5 @@ strictEqual(new acn().getFoo(), 2)
strictEqual(agfn().next().value, 2)
strictEqual(n, 2)
strictEqual(s, 'dogdawg')
strictEqual(callEx, 2)
strictEqual(somethingElse, 2)
19 changes: 19 additions & 0 deletions test/hook/duplicate-exports.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import * as lib from '../fixtures/duplicate.mjs'
import { notEqual, strictEqual } from 'assert'
import Hook from '../../index.js'

Hook((exports, name) => {
if (name.match(/duplicate\.mjs/)) {
// foo should not be exported because there are duplicate exports
strictEqual(exports.foo, undefined)
// default should be exported
strictEqual(exports.default, 'foo')
}
})

notEqual(lib, undefined)

// foo should not be exported because there are duplicate exports
strictEqual(lib.foo, undefined)
// default should be exported
strictEqual(lib.default, 'foo')
14 changes: 0 additions & 14 deletions test/hook/v18.19-static-import-gotalike.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,19 @@ 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'
}
})

/* eslint-disable import/no-named-default */
/* eslint-disable camelcase */
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')