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): Unwrapped wrapper helper functions
  • Loading branch information
danielgindi authored and lukastaegert committed Feb 24, 2021
commit ce489a15f5138e3dea8e2e0d84b1742d30e49e0d
2 changes: 1 addition & 1 deletion packages/commonjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
"@rollup/plugin-node-resolve": "^8.4.0",
"locate-character": "^2.0.5",
"require-relative": "^0.8.7",
"rollup": "^2.38.3",
"rollup": "^2.39.0",
"shx": "^0.3.2",
"source-map": "^0.7.3",
"source-map-support": "^0.5.19",
Expand Down
27 changes: 5 additions & 22 deletions packages/commonjs/src/dynamic-packages-manager.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
import { existsSync, readFileSync } from 'fs';
import { join } from 'path';

import {
DYNAMIC_PACKAGES_ID,
DYNAMIC_REGISTER_SUFFIX,
HELPERS_ID,
isWrappedId,
unwrapId,
wrapId
} from './helpers';
import { DYNAMIC_PACKAGES_ID, DYNAMIC_REGISTER_SUFFIX, HELPERS_ID, wrapId } from './helpers';
import { getVirtualPathForDynamicRequirePath, normalizePathSlashes } from './utils';

export function getPackageEntryPoint(dirPath) {
Expand Down Expand Up @@ -47,28 +40,18 @@ export function getDynamicPackagesEntryIntro(
) {
let dynamicImports = Array.from(
dynamicRequireModuleSet,
(dynamicId) => `require(${JSON.stringify(wrapModuleRegisterProxy(dynamicId))});`
(dynamicId) => `require(${JSON.stringify(wrapId(dynamicId, DYNAMIC_REGISTER_SUFFIX))});`
).join('\n');

if (dynamicRequireModuleDirPaths.length) {
dynamicImports += `require(${JSON.stringify(wrapModuleRegisterProxy(DYNAMIC_PACKAGES_ID))});`;
dynamicImports += `require(${JSON.stringify(
wrapId(DYNAMIC_PACKAGES_ID, DYNAMIC_REGISTER_SUFFIX)
)});`;
}

return dynamicImports;
}

export function wrapModuleRegisterProxy(id) {
return wrapId(id, DYNAMIC_REGISTER_SUFFIX);
}

export function unwrapModuleRegisterProxy(id) {
return unwrapId(id, DYNAMIC_REGISTER_SUFFIX);
}

export function isModuleRegisterProxy(id) {
return isWrappedId(id, DYNAMIC_REGISTER_SUFFIX);
}

export function isDynamicModuleImport(id, dynamicRequireModuleSet) {
const normalizedPath = normalizePathSlashes(id);
return dynamicRequireModuleSet.has(normalizedPath) && !normalizedPath.endsWith('.json');
Expand Down
22 changes: 10 additions & 12 deletions packages/commonjs/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@ import analyzeTopLevelStatements from './analyze-top-level-statements';
import {
getDynamicPackagesEntryIntro,
getDynamicPackagesModule,
isDynamicModuleImport,
isModuleRegisterProxy,
unwrapModuleRegisterProxy
isDynamicModuleImport
} from './dynamic-packages-manager';
import getDynamicRequirePaths from './dynamic-require-paths';
import {
DYNAMIC_JSON_PREFIX,
DYNAMIC_PACKAGES_ID,
DYNAMIC_REGISTER_SUFFIX,
EXPORTS_SUFFIX,
EXTERNAL_SUFFIX,
getHelpersModule,
Expand Down Expand Up @@ -123,12 +122,11 @@ export default function commonjs(options = {}) {
return { meta: { commonjs: { isCommonJS: false } } };
}

let disableWrap = false;

// avoid wrapping as this is a commonjsRegister call
if (isModuleRegisterProxy(id)) {
disableWrap = true;
id = unwrapModuleRegisterProxy(id);
const disableWrap = isWrappedId(id, DYNAMIC_REGISTER_SUFFIX);
if (disableWrap) {
// eslint-disable-next-line no-param-reassign
id = unwrapId(id, DYNAMIC_REGISTER_SUFFIX);
}

return transformCommonjs(
Expand Down Expand Up @@ -224,9 +222,9 @@ export default function commonjs(options = {}) {
return `export default require(${JSON.stringify(normalizePathSlashes(id))});`;
}

if (isModuleRegisterProxy(id)) {
if (isWrappedId(id, DYNAMIC_REGISTER_SUFFIX)) {
return getDynamicRequireProxy(
normalizePathSlashes(unwrapModuleRegisterProxy(id)),
normalizePathSlashes(unwrapId(id, DYNAMIC_REGISTER_SUFFIX)),
commonDir
);
}
Expand All @@ -247,8 +245,8 @@ export default function commonjs(options = {}) {
transform(code, rawId) {
let id = rawId;

if (isModuleRegisterProxy(id)) {
id = unwrapModuleRegisterProxy(id);
if (isWrappedId(id, DYNAMIC_REGISTER_SUFFIX)) {
id = unwrapId(id, DYNAMIC_REGISTER_SUFFIX);
}

const extName = extname(id);
Expand Down
23 changes: 10 additions & 13 deletions packages/commonjs/src/resolve-id.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { dirname, resolve, sep } from 'path';
import {
DYNAMIC_JSON_PREFIX,
DYNAMIC_PACKAGES_ID,
DYNAMIC_REGISTER_SUFFIX,
EXPORTS_SUFFIX,
EXTERNAL_SUFFIX,
HELPERS_ID,
Expand All @@ -16,11 +17,6 @@ import {
unwrapId,
wrapId
} from './helpers';
import {
isModuleRegisterProxy,
unwrapModuleRegisterProxy,
wrapModuleRegisterProxy
} from './dynamic-packages-manager';

function getCandidatesForExtension(resolved, extension) {
return [resolved + extension, `${resolved}${sep}index${extension}`];
Expand Down Expand Up @@ -54,14 +50,15 @@ export default function getResolveId(extensions) {
}

return function resolveId(importee, rawImporter) {
const importer =
rawImporter && isModuleRegisterProxy(rawImporter)
? unwrapModuleRegisterProxy(rawImporter)
: rawImporter;

if (isWrappedId(importee, MODULE_SUFFIX) || isWrappedId(importee, EXPORTS_SUFFIX)) {
return importee;
}

const importer =
rawImporter && isWrappedId(rawImporter, DYNAMIC_REGISTER_SUFFIX)
? unwrapId(rawImporter, DYNAMIC_REGISTER_SUFFIX)
: rawImporter;

// Except for exports, proxies are only importing resolved ids,
// no need to resolve again
if (importer && isWrappedId(importer, PROXY_SUFFIX)) {
Expand All @@ -77,9 +74,9 @@ export default function getResolveId(extensions) {
} else if (isRequiredModule) {
importee = unwrapId(importee, REQUIRE_SUFFIX);

isModuleRegistration = isModuleRegisterProxy(importee);
isModuleRegistration = isWrappedId(importee, DYNAMIC_REGISTER_SUFFIX);
if (isModuleRegistration) {
importee = unwrapModuleRegisterProxy(importee);
importee = unwrapId(importee, DYNAMIC_REGISTER_SUFFIX);
}
}

Expand All @@ -106,7 +103,7 @@ export default function getResolveId(extensions) {
resolved.id = wrapId(resolved.id, resolved.external ? EXTERNAL_SUFFIX : PROXY_SUFFIX);
resolved.external = false;
} else if (resolved && isModuleRegistration) {
resolved.id = wrapModuleRegisterProxy(resolved.id);
resolved.id = wrapId(resolved.id, DYNAMIC_REGISTER_SUFFIX);
} else if (!resolved && (isProxyModule || isRequiredModule)) {
return { id: wrapId(importee, EXTERNAL_SUFFIX), external: false };
}
Expand Down
20 changes: 10 additions & 10 deletions packages/commonjs/src/transform-commonjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,15 @@ import {
isRequireStatement,
isStaticRequireStatement
} from './generate-imports';
import { DYNAMIC_JSON_PREFIX } from './helpers';
import {
DYNAMIC_JSON_PREFIX,
DYNAMIC_REGISTER_SUFFIX,
isWrappedId,
unwrapId,
wrapId
} from './helpers';
import { tryParse } from './parse';
import { deconflict, getName, getVirtualPathForDynamicRequirePath } from './utils';
import {
isModuleRegisterProxy,
unwrapModuleRegisterProxy,
wrapModuleRegisterProxy
} from './dynamic-packages-manager';

const exportsPattern = /^(?:module\.)?exports(?:\.([a-zA-Z_$][a-zA-Z_$0-9]*))?$/;

Expand Down Expand Up @@ -230,13 +231,13 @@ export default function transformCommonjs(
}

let sourceId = getRequireStringArg(node);
const isDynamicRegister = isModuleRegisterProxy(sourceId);
const isDynamicRegister = isWrappedId(sourceId, DYNAMIC_REGISTER_SUFFIX);
if (isDynamicRegister) {
sourceId = unwrapModuleRegisterProxy(sourceId);
sourceId = unwrapId(sourceId, DYNAMIC_REGISTER_SUFFIX);
if (sourceId.endsWith('.json')) {
sourceId = DYNAMIC_JSON_PREFIX + sourceId;
}
dynamicRegisterSources.add(wrapModuleRegisterProxy(sourceId));
dynamicRegisterSources.add(wrapId(sourceId, DYNAMIC_REGISTER_SUFFIX));
} else {
if (
!sourceId.endsWith('.json') &&
Expand Down Expand Up @@ -332,7 +333,6 @@ export default function transformCommonjs(
storeName: true
});
}

usesDynamicRequire = true;
return;
case 'module':
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
var input = /*#__PURE__*/Object.defineProperty({
import * as commonjsHelpers from "_commonjsHelpers.js";
import { __exports as input } from "\u0000fixtures/form/compiled-esm-define-exports-empty/input.js?commonjs-exports"

}, '__esModule', {value: true});
Object.defineProperty(input, "__esModule", { value: true });

export default input;
export { input as __moduleExports };
export { input as __moduleExports, input as default };
2 changes: 1 addition & 1 deletion packages/commonjs/test/snapshots/function.js.md
Original file line number Diff line number Diff line change
Expand Up @@ -6371,7 +6371,7 @@ Generated by [AVA](https://avajs.dev).
/* eslint-disable */␊
var main = b ;␊
var main = b ;␊
module.exports = main;␊
`,
Expand Down
Binary file modified packages/commonjs/test/snapshots/function.js.snap
Binary file not shown.
41 changes: 25 additions & 16 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.