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

feat(commonjs): Add support for circular dependencies #658

Merged
merged 22 commits into from
May 7, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
5ec8891
feat(commonjs): Basic support for circular dependencies
lukastaegert Nov 24, 2020
0e41148
refactor(commonjs): Inline __esModule handling into export generation
lukastaegert Nov 24, 2020
a81058d
feat(commonjs): Generate simpler code when replacing module.exports
lukastaegert Nov 25, 2020
a3433fa
feat(commonjs): Handle side-effect free modules with updated Rollup
lukastaegert Jan 5, 2021
861d1ef
feat(commonjs): Generate simplified code when module is not used
lukastaegert Jan 21, 2021
5ac99ff
feat(commonjs): Do not use module when wrapping if not needed
lukastaegert Jan 23, 2021
5976ab3
chore(commonjs): Clean up usage of helpers
lukastaegert Jan 23, 2021
7ae3bab
feat(commonjs): Inline name export assignments
lukastaegert Jan 24, 2021
2d784b8
feat(commonjs): Do not wrap for nested module.exports assignments
lukastaegert Jan 24, 2021
b084933
feat(commonjs): Wrap if accessing exports while reassigning module.ex…
lukastaegert Jan 26, 2021
1bdaddc
chore(commonjs): Add test for module.exports mutations
lukastaegert Jan 26, 2021
a151e8d
feat(commonjs): Do not wrap for double exports assignments
lukastaegert Jan 28, 2021
e3c8f26
fix(commonjs): Deconflict nested module.exports assignments
lukastaegert Jan 28, 2021
ba6ffee
chore(commonjs): Simplify assignment tracking
lukastaegert Jan 29, 2021
1fcae9d
feat(commonjs): Do not wrap for nested named export assignments
lukastaegert Jan 29, 2021
f4f98bb
chore(commonjs): Clean up remaining TODOs
lukastaegert Jan 29, 2021
0a15527
fix(commonjs): Make sure require expressions return snapshots
lukastaegert Feb 1, 2021
fa817d5
fix(commonjs): Snapshot default export
lukastaegert Feb 1, 2021
ce489a1
chore(commonjs): Unwrapped wrapper helper functions
danielgindi Feb 15, 2021
e57b207
Merge branch 'master' into feat/commonjs-circular-dependencies
lukastaegert May 7, 2021
31d6298
chore: update dependencies
shellscape May 7, 2021
733147d
chore: change audit level to high amid unfixable moderate
shellscape May 7, 2021
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
Prev Previous commit
Next Next commit
feat(commonjs): Do not wrap for double exports assignments
  • Loading branch information
lukastaegert committed Feb 24, 2021
commit a151e8de5d4ee53695e719f6516b03daa65b12c0
15 changes: 9 additions & 6 deletions packages/commonjs/src/generate-exports.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,16 @@ export function rewriteExportsAndGetExportsBlock(
}

// Collect and rewrite named exports
for (const [exportName, node] of topLevelExportsAssignmentsByName) {
for (const [exportName, nodes] of topLevelExportsAssignmentsByName) {
// TODO Lukas if we allow nested assignments, we would need to deconflict all of them
const deconflicted = deconflict(exportName);
magicString.overwrite(
node.start,
node.left.end,
`var ${deconflicted} = ${exportsName}.${exportName}`
);
for (const node of nodes) {
magicString.overwrite(
node.start,
node.left.end,
`var ${deconflicted} = ${exportsName}.${exportName}`
);
}

if (exportName === 'default') {
deconflictedDefaultExportName = deconflicted;
Expand Down
10 changes: 3 additions & 7 deletions packages/commonjs/src/transform-commonjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,10 @@ export default function transformCommonjs(
shouldWrap = true;
} else if (exportName === KEY_COMPILED_ESM) {
topLevelDefineCompiledEsmExpressions.push(node);
} else if (!topLevelExportsAssignmentsByName.has(exportName)) {
topLevelExportsAssignmentsByName.set(exportName, node);
} else {
// TODO Lukas this is the case for multiple assignments to the same export
// we could handle this instead by collecting all assignments
// if there are only top-level assignments, turn the first to export var foo = ...;
// otherwise prefix the code with export var foo;
shouldWrap = true;
const exportAssignments = topLevelExportsAssignmentsByName.get(exportName) || [];
exportAssignments.push(node);
topLevelExportsAssignmentsByName.set(exportName, exportAssignments);
}

skippedNodes.add(node.left);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
description: 'handles double assignments to exports when using named imports'
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
exports.foo = 'first';
exports.foo = 'second';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { foo } from './dep';

t.is(foo, 'second');
20 changes: 16 additions & 4 deletions packages/commonjs/test/snapshots/function.js.md
Original file line number Diff line number Diff line change
Expand Up @@ -200,15 +200,27 @@ Generated by [AVA](https://avajs.dev).
var dep = {};␊
(function (exports) {␊
exports.foo = 'first';␊
exports.foo = 'second';␊
}(dep));␊
dep.foo = 'first';␊
dep.foo = 'second';␊
t.is(dep.foo, 'second');␊
`,
}

## double-exports-assignment-named

> Snapshot 1

{
'main.js': `'use strict';␊
var foo = 'first';␊
var foo = 'second';␊
t.is(foo, 'second');␊
`,
}

## double-module-exports-assignment

> Snapshot 1
Expand Down
Binary file modified packages/commonjs/test/snapshots/function.js.snap
Binary file not shown.