Skip to content

Commit

Permalink
fix(commonjs): idempotence issue (#871)
Browse files Browse the repository at this point in the history
  • Loading branch information
thorn0 authored May 4, 2021
1 parent 8298568 commit a843654
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 9 deletions.
8 changes: 5 additions & 3 deletions packages/commonjs/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export default function commonjs(options = {}) {

const esModulesWithDefaultExport = new Set();
const esModulesWithNamedExports = new Set();
const isCjsPromises = new Map();

const ignoreRequire =
typeof options.ignore === 'function'
Expand Down Expand Up @@ -208,7 +209,8 @@ export default function commonjs(options = {}) {
actualId,
getRequireReturnsDefault(actualId),
esModulesWithDefaultExport,
esModulesWithNamedExports
esModulesWithNamedExports,
isCjsPromises
);
}

Expand Down Expand Up @@ -244,11 +246,11 @@ export default function commonjs(options = {}) {
if (commonjs) {
const isCjs = commonjs.isCommonJS;
if (isCjs != null) {
setIsCjsPromise(id, isCjs);
setIsCjsPromise(isCjsPromises, id, isCjs);
return;
}
}
setIsCjsPromise(id, null);
setIsCjsPromise(isCjsPromises, id, null);
}
};
}
6 changes: 2 additions & 4 deletions packages/commonjs/src/is-cjs.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
const isCjsPromises = new Map();

export function getIsCjsPromise(id) {
export function getIsCjsPromise(isCjsPromises, id) {
let isCjsPromise = isCjsPromises.get(id);
if (isCjsPromise) return isCjsPromise.promise;

Expand All @@ -16,7 +14,7 @@ export function getIsCjsPromise(id) {
return promise;
}

export function setIsCjsPromise(id, resolution) {
export function setIsCjsPromise(isCjsPromises, id, resolution) {
const isCjsPromise = isCjsPromises.get(id);
if (isCjsPromise) {
if (isCjsPromise.resolve) {
Expand Down
5 changes: 3 additions & 2 deletions packages/commonjs/src/proxies.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,11 @@ export async function getStaticRequireProxy(
id,
requireReturnsDefault,
esModulesWithDefaultExport,
esModulesWithNamedExports
esModulesWithNamedExports,
isCjsPromises
) {
const name = getName(id);
const isCjs = await getIsCjsPromise(id);
const isCjs = await getIsCjsPromise(isCjsPromises, id);
if (isCjs) {
return `import { __moduleExports } from ${JSON.stringify(id)}; export default __moduleExports;`;
} else if (isCjs === null) {
Expand Down
15 changes: 15 additions & 0 deletions packages/commonjs/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -759,3 +759,18 @@ if (Number(/^v(\d+)/.exec(process.version)[1]) >= 12) {
t.is(code, await new Promise((done) => getRollupUpCodeWithCache.on('message', done)));
});
}

test('does not affect subsequently created instances when called with `requireReturnsDefault: "preferred"`', async (t) => {
const input = 'fixtures/function/import-esm-require-returns-default-preferred/main.js';
const options = { requireReturnsDefault: 'preferred' };

const instance1 = commonjs(options);
const bundle1 = await rollup({ input, plugins: [instance1] });
const code1 = (await bundle1.generate({})).output[0].code;

const instance2 = commonjs(options);
const bundle2 = await rollup({ input, plugins: [instance2] });
const code2 = (await bundle2.generate({})).output[0].code;

t.is(code1, code2);
});

0 comments on commit a843654

Please sign in to comment.