Skip to content

Commit

Permalink
feat(commonjs): automatically wrap cyclic modules (#1038)
Browse files Browse the repository at this point in the history
  • Loading branch information
lukastaegert committed Apr 24, 2022
1 parent e8dd3c3 commit 2b3d215
Show file tree
Hide file tree
Showing 49 changed files with 541 additions and 223 deletions.
30 changes: 18 additions & 12 deletions packages/commonjs/README.md
Expand Up @@ -44,13 +44,30 @@ Then call `rollup` either via the [CLI](https://www.rollupjs.org/guide/en/#comma

## Options

### `strictRequires`

Type: `"auto" | boolean | "debug" | string[]`<br>
Default: `"auto"`

By default, this plugin will try to hoist `require` statements as imports to the top of each file. While this works well for many code bases and allows for very efficient ESM output, it does not perfectly capture CommonJS semantics as the order of side effects like log statements may change. But it is especially problematic when there are circular `require` calls between CommonJS modules as those often rely on the lazy execution of nested `require` calls.

Setting this option to `true` will wrap all CommonJS files in functions which are executed when they are required for the first time, preserving NodeJS semantics. Note that this can have an impact on the size and performance of the generated code.

The default value of `"auto"` will only wrap CommonJS files when they are part of a CommonJS dependency cycle, e.g. an index file that is required by many of its dependencies. All other CommonJS files are hoisted. This is the recommended setting for most code bases.

`false` will entirely prevent wrapping and hoist all files. This may still work depending on the nature of cyclic dependencies but will often cause problems.

You can also provide a [minimatch pattern](https://github.com/isaacs/minimatch), or array of patterns, to only specify a subset of files which should be wrapped in functions for proper `require` semantics.

`"debug"` works like `"auto"` but after bundling, it will display a warning containing a list of ids that have been wrapped which can be used as minimatch pattern for fine-tuning.

### `dynamicRequireTargets`

Type: `string | string[]`<br>
Default: `[]`

Some modules contain dynamic `require` calls, or require modules that contain circular dependencies, which are not handled well by static imports.
Including those modules as `dynamicRequireTargets` will simulate a CommonJS (NodeJS-like) environment for them with support for dynamic and circular dependencies.
Including those modules as `dynamicRequireTargets` will simulate a CommonJS (NodeJS-like) environment for them with support for dynamic dependencies. It also enables `strictRequires` for those modules, see above.

_Note: In extreme cases, this feature may result in some paths being rendered as absolute in the final bundle. The plugin tries to avoid exposing paths from the local machine, but if you are `dynamicRequirePaths` with paths that are far away from your project's folder, that may require replacing strings like `"/Users/John/Desktop/foo-project/"` -> `"/"`._

Expand Down Expand Up @@ -113,17 +130,6 @@ Default: `false`

Instructs the plugin whether to enable mixed module transformations. This is useful in scenarios with modules that contain a mix of ES `import` statements and CommonJS `require` expressions. Set to `true` if `require` calls should be transformed to imports in mixed modules, or `false` if the `require` expressions should survive the transformation. The latter can be important if the code contains environment detection, or you are coding for an environment with special treatment for `require` calls such as [ElectronJS](https://www.electronjs.org/). See also the "ignore" option.

### `strictRequireSemantic`

Type: `boolean | string | string[]`<br>
Default: `false`

By default, this plugin will try to hoist all `require` statements as imports to the top of each file. While this works well for many code bases and allows for very efficient ESM output, it does not perfectly capture CommonJS semantics. This is especially problematic when there are circular `require` calls between CommonJS modules as those often rely on the lazy execution of nested `require` calls.

Setting this option to `true` will wrap all CommonJS files in functions which are executed when they are required for the first time, preserving NodeJS semantics. Note that this can have a small impact on the size and performance of the generated code.

You can also provide a [minimatch pattern](https://github.com/isaacs/minimatch), or array of patterns, to only specify a subset of files which should be wrapped in functions for proper `require` semantics.

### `ignore`

Type: `string[] | ((id: string) => boolean)`<br>
Expand Down
11 changes: 7 additions & 4 deletions packages/commonjs/src/generate-imports.js
Expand Up @@ -112,7 +112,7 @@ export function getRequireHandlers() {
id,
exportMode,
resolveRequireSourcesAndGetMeta,
usesRequireWrapper,
needsRequireWrapper,
isEsModule,
usesRequire
) {
Expand All @@ -135,13 +135,16 @@ export function getRequireHandlers() {
);
}
const requiresBySource = collectSources(requireExpressions);
const requireTargets = await resolveRequireSourcesAndGetMeta(
const { requireTargets, usesRequireWrapper } = await resolveRequireSourcesAndGetMeta(
id,
usesRequireWrapper ? IS_WRAPPED_COMMONJS : !isEsModule,
needsRequireWrapper ? IS_WRAPPED_COMMONJS : !isEsModule,
Object.keys(requiresBySource)
);
processRequireExpressions(imports, requireTargets, requiresBySource, magicString);
return imports.length ? `${imports.join('\n')}\n\n` : '';
return {
importBlock: imports.length ? `${imports.join('\n')}\n\n` : '',
usesRequireWrapper
};
}

return {
Expand Down
44 changes: 32 additions & 12 deletions packages/commonjs/src/index.js
@@ -1,4 +1,4 @@
import { extname } from 'path';
import { extname, relative } from 'path';

import { createFilter } from '@rollup/pluginutils';
import getCommonDir from 'commondir';
Expand Down Expand Up @@ -27,7 +27,7 @@ import getResolveId from './resolve-id';
import { getResolveRequireSourcesAndGetMeta } from './resolve-require-sources';
import validateRollupVersion from './rollup-version';
import transformCommonjs from './transform-commonjs';
import { getName, normalizePathSlashes } from './utils';
import { getName, getStrictRequiresFilter, normalizePathSlashes } from './utils';

export default function commonjs(options = {}) {
const {
Expand All @@ -39,12 +39,7 @@ export default function commonjs(options = {}) {
} = options;
const extensions = options.extensions || ['.js'];
const filter = createFilter(options.include, options.exclude);
const strictRequireSemanticFilter =
options.strictRequireSemantic === true
? () => true
: !options.strictRequireSemantic
? () => false
: createFilter(options.strictRequireSemantic);
const { strictRequiresFilter, detectCycles } = getStrictRequiresFilter(options);

const getRequireReturnsDefault =
typeof requireReturnsDefaultOption === 'function'
Expand All @@ -65,7 +60,10 @@ export default function commonjs(options = {}) {
: () =>
typeof defaultIsModuleExportsOption === 'boolean' ? defaultIsModuleExportsOption : 'auto';

const resolveRequireSourcesAndGetMeta = getResolveRequireSourcesAndGetMeta(extensions);
const { resolveRequireSourcesAndGetMeta, getWrappedIds } = getResolveRequireSourcesAndGetMeta(
extensions,
detectCycles
);
const dynamicRequireModuleSet = getDynamicRequireModuleSet(options.dynamicRequireTargets);
const isDynamicRequireModulesEnabled = dynamicRequireModuleSet.size > 0;
const commonDir = isDynamicRequireModulesEnabled
Expand Down Expand Up @@ -122,9 +120,9 @@ export default function commonjs(options = {}) {
return { meta: { commonjs: { isCommonJS: false } } };
}

const usesRequireWrapper =
const needsRequireWrapper =
!isEsModule &&
(dynamicRequireModuleSet.has(normalizePathSlashes(id)) || strictRequireSemanticFilter(id));
(dynamicRequireModuleSet.has(normalizePathSlashes(id)) || strictRequiresFilter(id));

return transformCommonjs(
this.parse,
Expand All @@ -141,7 +139,7 @@ export default function commonjs(options = {}) {
commonDir,
ast,
getDefaultIsModuleExports(id),
usesRequireWrapper,
needsRequireWrapper,
resolveRequireSourcesAndGetMeta(this)
);
}
Expand All @@ -158,6 +156,28 @@ export default function commonjs(options = {}) {
}
},

buildEnd() {
if (options.strictRequires === 'debug') {
const wrappedIds = getWrappedIds();
if (wrappedIds.length) {
this.warn({
code: 'WRAPPED_IDS',
ids: wrappedIds,
message: `The commonjs plugin automatically wrapped the following files:\n[\n${wrappedIds
.map((id) => `\t${JSON.stringify(relative(process.cwd(), id))}`)
.join(',\n')}\n]`
});
} else {
// TODO Lukas test
this.warn({
code: 'WRAPPED_IDS',
ids: wrappedIds,
message: 'The commonjs plugin did not wrap any files.'
});
}
}
},

resolveId,

load(id) {
Expand Down
114 changes: 70 additions & 44 deletions packages/commonjs/src/resolve-require-sources.js
@@ -1,54 +1,80 @@
import { EXTERNAL_SUFFIX, IS_WRAPPED_COMMONJS, PROXY_SUFFIX, wrapId } from './helpers';
import { resolveExtensions } from './resolve-id';

// TODO Lukas auto-detect circular dependencies
// * only return once all dependencies have been analyzed
// * wait for this.load dependencies unless they already have an entry in knownCjsModuleTypes to avoid deadlocks
// as those have already started being processed
// * only analyze cycles if we do not have an explicit config
export function getResolveRequireSourcesAndGetMeta(extensions) {
export function getResolveRequireSourcesAndGetMeta(extensions, detectCycles) {
const knownCjsModuleTypes = Object.create(null);
return (rollupContext) => (id, isParentCommonJS, sources) => {
knownCjsModuleTypes[id] = isParentCommonJS;
return Promise.all(
sources.map(async (source) => {
// Never analyze or proxy internal modules
if (source.startsWith('\0')) {
return { source, id: source, isCommonJS: false };
}
const resolved =
(await rollupContext.resolve(source, id, {
skipSelf: true,
custom: {
'node-resolve': { isRequire: true }
const dependentModules = Object.create(null);
const getDependentModules = (id) =>
dependentModules[id] || (dependentModules[id] = Object.create(null));

return {
getWrappedIds: () =>
Object.keys(knownCjsModuleTypes).filter(
(id) => knownCjsModuleTypes[id] === IS_WRAPPED_COMMONJS
),
resolveRequireSourcesAndGetMeta: (rollupContext) => async (id, isParentCommonJS, sources) => {
knownCjsModuleTypes[id] = isParentCommonJS;
const requireTargets = await Promise.all(
sources.map(async (source) => {
// Never analyze or proxy internal modules
if (source.startsWith('\0')) {
return { id: source, allowProxy: false };
}
const resolved =
(await rollupContext.resolve(source, id, {
skipSelf: true,
custom: {
'node-resolve': { isRequire: true }
}
})) || resolveExtensions(source, id, extensions);
if (!resolved) {
return { id: wrapId(source, EXTERNAL_SUFFIX), allowProxy: false };
}
const childId = resolved.id;
if (resolved.external) {
return { id: wrapId(childId, EXTERNAL_SUFFIX), allowProxy: false };
}
const parentDependentModules = getDependentModules(id);
const childDependentModules = getDependentModules(childId);
childDependentModules[id] = true;
for (const dependentId of Object.keys(parentDependentModules)) {
childDependentModules[dependentId] = true;
}
if (parentDependentModules[childId]) {
// If we depend on one of our dependencies, we have a cycle. Then all modules that
// we depend on that also depend on the same module are part of a cycle as well.
if (detectCycles && isParentCommonJS) {
knownCjsModuleTypes[id] = IS_WRAPPED_COMMONJS;
knownCjsModuleTypes[childId] = IS_WRAPPED_COMMONJS;
for (const dependentId of Object.keys(parentDependentModules)) {
if (getDependentModules(dependentId)[childId]) {
knownCjsModuleTypes[dependentId] = IS_WRAPPED_COMMONJS;
}
}
}
})) || resolveExtensions(source, id, extensions);
if (!resolved) {
return { source, id: wrapId(source, EXTERNAL_SUFFIX), isCommonJS: false };
}
if (resolved.external) {
return { source, id: wrapId(resolved.id, EXTERNAL_SUFFIX), isCommonJS: false };
}
if (resolved.id in knownCjsModuleTypes) {
} else {
// This makes sure the current transform handler waits for all direct dependencies to be
// loaded and transformed and therefore for all transitive CommonJS dependencies to be
// loaded as well so that all cycles have been found and knownCjsModuleTypes is reliable.
await rollupContext.load(resolved);
}
return { id: childId, allowProxy: true };
})
);
return {
requireTargets: requireTargets.map(({ id: dependencyId, allowProxy }, index) => {
const isCommonJS = knownCjsModuleTypes[dependencyId];
return {
source,
source: sources[index],
id:
knownCjsModuleTypes[resolved.id] === IS_WRAPPED_COMMONJS
? resolved.id
: wrapId(resolved.id, PROXY_SUFFIX),
isCommonJS: knownCjsModuleTypes[resolved.id]
allowProxy && isCommonJS !== IS_WRAPPED_COMMONJS
? wrapId(dependencyId, PROXY_SUFFIX)
: dependencyId,
isCommonJS
};
}
const {
meta: { commonjs: commonjsMeta }
} = await rollupContext.load(resolved);
const isCommonJS = commonjsMeta && commonjsMeta.isCommonJS;
return {
source,
id: isCommonJS === IS_WRAPPED_COMMONJS ? resolved.id : wrapId(resolved.id, PROXY_SUFFIX),
isCommonJS
};
})
);
}),
usesRequireWrapper: knownCjsModuleTypes[id] === IS_WRAPPED_COMMONJS
};
}
};
}
23 changes: 11 additions & 12 deletions packages/commonjs/src/transform-commonjs.js
Expand Up @@ -50,7 +50,7 @@ export default async function transformCommonjs(
commonDir,
astCache,
defaultIsModuleExports,
usesRequireWrapper,
needsRequireWrapper,
resolveRequireSourcesAndGetMeta
) {
const ast = astCache || tryParse(parse, code, id);
Expand Down Expand Up @@ -227,9 +227,10 @@ export default async function transformCommonjs(
let shouldRemoveRequireStatement = false;

if (currentTryBlockEnd !== null) {
({ canConvertRequire, shouldRemoveRequireStatement } =
getIgnoreTryCatchRequireStatementMode(node.arguments[0].value));

const ignoreTryCatchRequire = getIgnoreTryCatchRequireStatementMode(
node.arguments[0].value
);
({ canConvertRequire, shouldRemoveRequireStatement } = ignoreTryCatchRequire);
if (shouldRemoveRequireStatement) {
hasRemovedRequire = true;
}
Expand Down Expand Up @@ -447,7 +448,7 @@ export default async function transformCommonjs(
? 'exports'
: 'module';

const importBlock = await rewriteRequireExpressionsAndGetImportBlock(
const { importBlock, usesRequireWrapper } = await rewriteRequireExpressionsAndGetImportBlock(
magicString,
topLevelDeclarations,
topLevelRequireDeclarators,
Expand All @@ -459,7 +460,7 @@ export default async function transformCommonjs(
id,
exportMode,
resolveRequireSourcesAndGetMeta,
usesRequireWrapper,
needsRequireWrapper,
isEsModule,
uses.require
);
Expand Down Expand Up @@ -490,17 +491,15 @@ export default async function transformCommonjs(
}

if (usesRequireWrapper) {
magicString
.trim()
.indent('\t')
.prepend(
`var ${isRequiredName};
magicString.trim().indent('\t');
magicString.prepend(
`var ${isRequiredName};
function ${requireName} () {
\tif (${isRequiredName}) return ${exportsName};
\t${isRequiredName} = 1;
`
).append(`
).append(`
\treturn ${exportsName};
}`);
if (exportMode === 'replace') {
Expand Down

0 comments on commit 2b3d215

Please sign in to comment.