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
chore(commonjs): Clean up usage of helpers
  • Loading branch information
lukastaegert committed Feb 24, 2021
commit 5976ab38c54f742b12b2f639c2bd1d140cd11388
15 changes: 3 additions & 12 deletions packages/commonjs/src/generate-exports.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ export function rewriteExportsAndGetExportsBlock(
defineCompiledEsmExpressions,
deconflict,
code,
uses,
HELPERS_NAME,
exportMode
exportMode,
detectWrappedDefault
) {
const exports = [];
const exportDeclarations = [];
Expand All @@ -41,10 +41,7 @@ export function rewriteExportsAndGetExportsBlock(
} else {
exports.push(`${exportsName} as __moduleExports`);
if (wrapped) {
if (defineCompiledEsmExpressions.length > 0 || code.indexOf('__esModule') >= 0) {
// TODO Lukas this can have no effect, is it covered? Inline?
// eslint-disable-next-line no-param-reassign
uses.commonjsHelpers = true;
if (detectWrappedDefault) {
exportDeclarations.push(
`export default /*@__PURE__*/${HELPERS_NAME}.getDefaultExportFromCjs(${exportsName});`
);
Expand Down Expand Up @@ -102,12 +99,6 @@ export function rewriteExportsAndGetExportsBlock(

if (isRestorableCompiledEsm) {
exports.push(`${deconflictedDefaultExportName || exportsName} as default`);
} else if (deconflictedDefaultExportName && code.indexOf('__esModule') >= 0) {
// eslint-disable-next-line no-param-reassign
uses.commonjsHelpers = true;
exportDeclarations.push(
`export default /*@__PURE__*/${HELPERS_NAME}.getDefaultExportFromCjs(${exportsName});`
);
} else {
exports.push(`${exportsName} as default`);
}
Expand Down
33 changes: 18 additions & 15 deletions packages/commonjs/src/transform-commonjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@ export default function transformCommonjs(
module: false,
exports: false,
global: false,
require: false,
commonjsHelpers: false
require: false
};
let usesCommonjsHelpers = false;
const virtualDynamicRequirePath =
isDynamicRequireModulesEnabled && getVirtualPathForDynamicRequirePath(dirname(id), commonDir);
let scope = attachScopes(ast, 'scope');
Expand Down Expand Up @@ -213,7 +213,7 @@ export default function transformCommonjs(
storeName: true
}
);
uses.commonjsHelpers = true;
usesCommonjsHelpers = true;
return;
}

Expand Down Expand Up @@ -264,7 +264,7 @@ export default function transformCommonjs(
dirname(id) === '.' ? null /* default behavior */ : virtualDynamicRequirePath
)})`
);
uses.commonjsHelpers = true;
usesCommonjsHelpers = true;
}
return;
}
Expand Down Expand Up @@ -321,7 +321,7 @@ export default function transformCommonjs(
magicString.overwrite(node.start, node.end, `${HELPERS_NAME}.commonjsRequire`, {
storeName: true
});
uses.commonjsHelpers = true;
usesCommonjsHelpers = true;
}
}

Expand All @@ -344,7 +344,7 @@ export default function transformCommonjs(
});
}

uses.commonjsHelpers = true;
usesCommonjsHelpers = true;
return;
case 'module':
case 'exports':
Expand All @@ -357,7 +357,7 @@ export default function transformCommonjs(
magicString.overwrite(node.start, node.end, `${HELPERS_NAME}.commonjsGlobal`, {
storeName: true
});
uses.commonjsHelpers = true;
usesCommonjsHelpers = true;
}
return;
case 'define':
Expand All @@ -375,7 +375,7 @@ export default function transformCommonjs(
magicString.overwrite(node.start, node.end, `${HELPERS_NAME}.commonjsRequire`, {
storeName: true
});
uses.commonjsHelpers = true;
usesCommonjsHelpers = true;
skippedNodes.add(node.object);
skippedNodes.add(node.property);
}
Expand All @@ -394,7 +394,7 @@ export default function transformCommonjs(
magicString.overwrite(node.start, node.end, `${HELPERS_NAME}.commonjsGlobal`, {
storeName: true
});
uses.commonjsHelpers = true;
usesCommonjsHelpers = true;
}
}
return;
Expand Down Expand Up @@ -438,7 +438,10 @@ export default function transformCommonjs(

// We cannot wrap ES/mixed modules
shouldWrap = shouldWrap && !disableWrap && !isEsModule;
uses.commonjsHelpers = uses.commonjsHelpers || shouldWrap;
const detectWrappedDefault =
shouldWrap &&
(topLevelDefineCompiledEsmExpressions.length > 0 || code.indexOf('__esModule') >= 0);
usesCommonjsHelpers = usesCommonjsHelpers || detectWrappedDefault;

if (
!(
Expand All @@ -447,9 +450,9 @@ export default function transformCommonjs(
uses.module ||
uses.exports ||
uses.require ||
uses.commonjsHelpers ||
usesCommonjsHelpers ||
hasRemovedRequire ||
isRestorableCompiledEsm
topLevelDefineCompiledEsmExpressions.length > 0
) &&
(ignoreGlobal || !uses.global)
) {
Expand Down Expand Up @@ -479,7 +482,7 @@ export default function transformCommonjs(
topLevelDeclarations,
topLevelRequireDeclarators,
reassignedNames,
uses.commonjsHelpers && HELPERS_NAME,
usesCommonjsHelpers && HELPERS_NAME,
dynamicRegisterSources,
moduleName,
exportsName,
Expand All @@ -499,9 +502,9 @@ export default function transformCommonjs(
topLevelDefineCompiledEsmExpressions,
(name) => deconflict(scope, globals, name),
code,
uses,
HELPERS_NAME,
exportMode
exportMode,
detectWrappedDefault
);

if (shouldWrap) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as commonjsHelpers from "_commonjsHelpers.js";
import { __module as inputModule, exports as input } from "\u0000fixtures/form/typeof-module-exports/input.js?commonjs-module"

(function (module, exports) {
Expand Down
2 changes: 1 addition & 1 deletion packages/commonjs/test/form.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ fs.readdirSync('./fixtures/form').forEach((dir) => {
const actual = (transformed ? transformed.code : input).trim().replace(/\0/g, '_');

// uncomment to update snapshots
fs.writeFileSync(outputFile, `${actual}\n`);
// fs.writeFileSync(outputFile, `${actual}\n`);

// trim whitespace from line endings,
// this will benefit issues like `form/try-catch-remove` where whitespace is left in the line,
Expand Down