diff --git a/packages/commonjs/src/index.js b/packages/commonjs/src/index.js index 1d0fdcc59..627f98eb6 100644 --- a/packages/commonjs/src/index.js +++ b/packages/commonjs/src/index.js @@ -128,16 +128,19 @@ export default function commonjs(options = {}) { !isEsModule && (dynamicRequireModules.has(normalizePathSlashes(id)) || strictRequiresFilter(id)); - const checkDynamicRequire = () => { + const checkDynamicRequire = (position) => { if (id.indexOf(dynamicRequireRoot) !== 0) { - this.error({ - code: 'DYNAMIC_REQUIRE_OUTSIDE_ROOT', - id, - dynamicRequireRoot, - message: `"${id}" contains dynamic require statements but it is not within the current dynamicRequireRoot "${dynamicRequireRoot}". You should set dynamicRequireRoot to "${dirname( - id - )}" or one of its parent directories.` - }); + this.error( + { + code: 'DYNAMIC_REQUIRE_OUTSIDE_ROOT', + id, + dynamicRequireRoot, + message: `"${id}" contains dynamic require statements but it is not within the current dynamicRequireRoot "${dynamicRequireRoot}". You should set dynamicRequireRoot to "${dirname( + id + )}" or one of its parent directories.` + }, + position + ); } }; diff --git a/packages/commonjs/src/resolve-id.js b/packages/commonjs/src/resolve-id.js index c7d3844a9..74d209e5b 100644 --- a/packages/commonjs/src/resolve-id.js +++ b/packages/commonjs/src/resolve-id.js @@ -50,6 +50,14 @@ export function resolveExtensions(importee, importer, extensions) { export default function getResolveId(extensions) { return async function resolveId(importee, importer, resolveOptions) { + // We assume that all requires are pre-resolved + if ( + resolveOptions.custom && + resolveOptions.custom['node-resolve'] && + resolveOptions.custom['node-resolve'].isRequire + ) { + return null; + } if (isWrappedId(importee, WRAPPED_SUFFIX)) { return unwrapId(importee, WRAPPED_SUFFIX); } @@ -69,7 +77,7 @@ export default function getResolveId(extensions) { if (importer) { if ( importer === DYNAMIC_MODULES_ID || - // Except for exports, proxies are only importing resolved ids, no need to resolve again + // Proxies are only importing resolved ids, no need to resolve again isWrappedId(importer, PROXY_SUFFIX) || isWrappedId(importer, ES_IMPORT_SUFFIX) ) { diff --git a/packages/commonjs/src/resolve-require-sources.js b/packages/commonjs/src/resolve-require-sources.js index 9483d2cd1..76fae132e 100644 --- a/packages/commonjs/src/resolve-require-sources.js +++ b/packages/commonjs/src/resolve-require-sources.js @@ -11,9 +11,39 @@ export function getResolveRequireSourcesAndGetMeta(extensions, detectCyclesAndCo const knownCjsModuleTypes = Object.create(null); const requiredIds = Object.create(null); const unconditionallyRequiredIds = Object.create(null); - const dependentModules = Object.create(null); - const getDependentModules = (id) => - dependentModules[id] || (dependentModules[id] = Object.create(null)); + const dependencies = Object.create(null); + const getDependencies = (id) => dependencies[id] || (dependencies[id] = new Set()); + + const isCyclic = (id) => { + const dependenciesToCheck = new Set(getDependencies(id)); + for (const dependency of dependenciesToCheck) { + if (dependency === id) { + return true; + } + for (const childDependency of getDependencies(dependency)) { + dependenciesToCheck.add(childDependency); + } + } + return false; + }; + + const fullyAnalyzedModules = Object.create(null); + + const getTypeForFullyAnalyzedModule = (id) => { + const knownType = knownCjsModuleTypes[id]; + if ( + knownType === IS_WRAPPED_COMMONJS || + !detectCyclesAndConditional || + fullyAnalyzedModules[id] + ) { + return knownType; + } + fullyAnalyzedModules[id] = true; + if (isCyclic(id)) { + return (knownCjsModuleTypes[id] = IS_WRAPPED_COMMONJS); + } + return knownType; + }; return { getWrappedIds: () => @@ -26,8 +56,9 @@ export function getResolveRequireSourcesAndGetMeta(extensions, detectCyclesAndCo isParentCommonJS, sources ) => { - knownCjsModuleTypes[parentId] = knownCjsModuleTypes[parentId] || isParentCommonJS; + knownCjsModuleTypes[parentId] = isParentCommonJS; if ( + detectCyclesAndConditional && knownCjsModuleTypes[parentId] && requiredIds[parentId] && !unconditionallyRequiredIds[parentId] @@ -42,9 +73,7 @@ export function getResolveRequireSourcesAndGetMeta(extensions, detectCyclesAndCo } const resolved = (await rollupContext.resolve(source, parentId, { - custom: { - 'node-resolve': { isRequire: true } - } + custom: { 'node-resolve': { isRequire: true } } })) || resolveExtensions(source, parentId, extensions); if (!resolved) { return { id: wrapId(source, EXTERNAL_SUFFIX), allowProxy: false }; @@ -54,44 +83,25 @@ export function getResolveRequireSourcesAndGetMeta(extensions, detectCyclesAndCo return { id: wrapId(childId, EXTERNAL_SUFFIX), allowProxy: false }; } requiredIds[childId] = true; - if ( - !( - detectCyclesAndConditional && - (isConditional || knownCjsModuleTypes[parentId] === IS_WRAPPED_COMMONJS) - ) - ) { + if (!(isConditional || knownCjsModuleTypes[parentId] === IS_WRAPPED_COMMONJS)) { unconditionallyRequiredIds[childId] = true; } - const parentDependentModules = getDependentModules(parentId); - const childDependentModules = getDependentModules(childId); - childDependentModules[parentId] = 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 (detectCyclesAndConditional && isParentCommonJS) { - knownCjsModuleTypes[parentId] = IS_WRAPPED_COMMONJS; - knownCjsModuleTypes[childId] = IS_WRAPPED_COMMONJS; - for (const dependentId of Object.keys(parentDependentModules)) { - if (getDependentModules(dependentId)[childId]) { - knownCjsModuleTypes[dependentId] = IS_WRAPPED_COMMONJS; - } - } - } - } else { + + getDependencies(parentId).add(childId); + if (!isCyclic(childId)) { // 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); + } else if (detectCyclesAndConditional && knownCjsModuleTypes[parentId]) { + knownCjsModuleTypes[parentId] = IS_WRAPPED_COMMONJS; } return { id: childId, allowProxy: true }; }) ); return { requireTargets: requireTargets.map(({ id: dependencyId, allowProxy }, index) => { - const isCommonJS = knownCjsModuleTypes[dependencyId]; + const isCommonJS = getTypeForFullyAnalyzedModule(dependencyId); return { source: sources[index].source, id: allowProxy @@ -102,7 +112,7 @@ export function getResolveRequireSourcesAndGetMeta(extensions, detectCyclesAndCo isCommonJS }; }), - usesRequireWrapper: knownCjsModuleTypes[parentId] === IS_WRAPPED_COMMONJS + usesRequireWrapper: getTypeForFullyAnalyzedModule(parentId) === IS_WRAPPED_COMMONJS }; } }; diff --git a/packages/commonjs/src/transform-commonjs.js b/packages/commonjs/src/transform-commonjs.js index e4147d4cd..3fb8602cd 100644 --- a/packages/commonjs/src/transform-commonjs.js +++ b/packages/commonjs/src/transform-commonjs.js @@ -198,7 +198,7 @@ export default async function transformCommonjs( isRequire(node.callee.object, scope) && node.callee.property.name === 'resolve' ) { - checkDynamicRequire(); + checkDynamicRequire(node.start); uses.require = true; const requireNode = node.callee.object; magicString.appendLeft( @@ -220,6 +220,7 @@ export default async function transformCommonjs( if (hasDynamicArguments(node)) { if (isDynamicRequireModulesEnabled) { + checkDynamicRequire(node.start); magicString.appendLeft( node.end - 1, `, ${JSON.stringify( @@ -228,7 +229,6 @@ export default async function transformCommonjs( ); } if (!ignoreDynamicRequires) { - checkDynamicRequire(); replacedDynamicRequires.push(node.callee); } return; @@ -286,7 +286,6 @@ export default async function transformCommonjs( return; } if (!ignoreDynamicRequires) { - checkDynamicRequire(); if (isShorthandProperty(parent)) { magicString.prependRight(node.start, 'require: '); } @@ -370,9 +369,10 @@ export default async function transformCommonjs( if (scope.contains(flattened.name)) return; if ( - flattened.keypath === 'module.exports' || - flattened.keypath === 'module' || - flattened.keypath === 'exports' + !isEsModule && + (flattened.keypath === 'module.exports' || + flattened.keypath === 'module' || + flattened.keypath === 'exports') ) { magicString.overwrite(node.start, node.end, `'object'`, { storeName: false diff --git a/packages/commonjs/test/fixtures/function/load-cycle-parallel/_config.js b/packages/commonjs/test/fixtures/function/load-cycle-parallel/_config.js new file mode 100644 index 000000000..73cd1001e --- /dev/null +++ b/packages/commonjs/test/fixtures/function/load-cycle-parallel/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'handles loading all modules of a cycle in parallel' +}; diff --git a/packages/commonjs/test/fixtures/function/load-cycle-parallel/a.js b/packages/commonjs/test/fixtures/function/load-cycle-parallel/a.js new file mode 100644 index 000000000..ed897bbc6 --- /dev/null +++ b/packages/commonjs/test/fixtures/function/load-cycle-parallel/a.js @@ -0,0 +1 @@ +require('./b.js'); diff --git a/packages/commonjs/test/fixtures/function/load-cycle-parallel/b.js b/packages/commonjs/test/fixtures/function/load-cycle-parallel/b.js new file mode 100644 index 000000000..60204ed1e --- /dev/null +++ b/packages/commonjs/test/fixtures/function/load-cycle-parallel/b.js @@ -0,0 +1 @@ +require('./c.js'); diff --git a/packages/commonjs/test/fixtures/function/load-cycle-parallel/c.js b/packages/commonjs/test/fixtures/function/load-cycle-parallel/c.js new file mode 100644 index 000000000..d87ec6cce --- /dev/null +++ b/packages/commonjs/test/fixtures/function/load-cycle-parallel/c.js @@ -0,0 +1 @@ +require('./a.js'); diff --git a/packages/commonjs/test/fixtures/function/load-cycle-parallel/main.js b/packages/commonjs/test/fixtures/function/load-cycle-parallel/main.js new file mode 100644 index 000000000..c6d056017 --- /dev/null +++ b/packages/commonjs/test/fixtures/function/load-cycle-parallel/main.js @@ -0,0 +1,3 @@ +require('./a.js'); +require('./b.js'); +require('./c.js'); diff --git a/packages/commonjs/test/fixtures/samples/mixed-module-typeof-exports/_config.js b/packages/commonjs/test/fixtures/samples/mixed-module-typeof-exports/_config.js new file mode 100644 index 000000000..3d25d8d4d --- /dev/null +++ b/packages/commonjs/test/fixtures/samples/mixed-module-typeof-exports/_config.js @@ -0,0 +1,4 @@ +module.exports = { + description: 'replaces "typeof exports" with "undefined" in mixed modules', + pluginOptions: { transformMixedEsModules: true } +}; diff --git a/packages/commonjs/test/fixtures/samples/mixed-module-typeof-exports/foo.js b/packages/commonjs/test/fixtures/samples/mixed-module-typeof-exports/foo.js new file mode 100644 index 000000000..ce0fffb75 --- /dev/null +++ b/packages/commonjs/test/fixtures/samples/mixed-module-typeof-exports/foo.js @@ -0,0 +1 @@ +module.exports = 21; diff --git a/packages/commonjs/test/fixtures/samples/mixed-module-typeof-exports/main.js b/packages/commonjs/test/fixtures/samples/mixed-module-typeof-exports/main.js new file mode 100644 index 000000000..b1f34e2a2 --- /dev/null +++ b/packages/commonjs/test/fixtures/samples/mixed-module-typeof-exports/main.js @@ -0,0 +1,7 @@ +const foo = require('./foo'); + +if (typeof exports !== 'undefined') { + throw new Error('There should be no global exports in an ES module'); +} + +export { foo as default }; diff --git a/packages/commonjs/test/snapshots/function.js.md b/packages/commonjs/test/snapshots/function.js.md index b30e1811d..70584b812 100644 --- a/packages/commonjs/test/snapshots/function.js.md +++ b/packages/commonjs/test/snapshots/function.js.md @@ -3050,8 +3050,10 @@ Generated by [AVA](https://avajs.dev). root = window;␊ } else if (typeof global !== 'undefined') {␊ root = global;␊ - } else {␊ + } else if (typeof module !== 'undefined') {␊ root = module;␊ + } else {␊ + root = Function('return this')(); // eslint-disable-line no-new-func␊ }␊ ␊ root.pollution = 'foo';␊ @@ -4725,6 +4727,19 @@ Generated by [AVA](https://avajs.dev). `, } +## load-cycle-parallel + +> Snapshot 1 + + { + 'main.js': `'use strict';␊ + ␊ + var main = {};␊ + ␊ + module.exports = main;␊ + `, + } + ## module-meta-properties > Snapshot 1 diff --git a/packages/commonjs/test/snapshots/function.js.snap b/packages/commonjs/test/snapshots/function.js.snap index a4ae4968c..9a637b0f7 100644 Binary files a/packages/commonjs/test/snapshots/function.js.snap and b/packages/commonjs/test/snapshots/function.js.snap differ diff --git a/packages/commonjs/test/snapshots/test.js.md b/packages/commonjs/test/snapshots/test.js.md index 86e1a2048..1b1ebe464 100644 --- a/packages/commonjs/test/snapshots/test.js.md +++ b/packages/commonjs/test/snapshots/test.js.md @@ -100,3 +100,18 @@ Generated by [AVA](https://avajs.dev). ␊ module.exports = main;␊ ` + +## does not transform typeof exports for mixed modules + +> Snapshot 1 + + `var foo$1 = 21;␊ + ␊ + const foo = foo$1;␊ + ␊ + if (typeof exports !== 'undefined') {␊ + throw new Error('There should be no global exports in an ES module');␊ + }␊ + ␊ + export { foo as default };␊ + ` diff --git a/packages/commonjs/test/snapshots/test.js.snap b/packages/commonjs/test/snapshots/test.js.snap index d685942af..685297264 100644 Binary files a/packages/commonjs/test/snapshots/test.js.snap and b/packages/commonjs/test/snapshots/test.js.snap differ diff --git a/packages/commonjs/test/test.js b/packages/commonjs/test/test.js index ea0afb695..097f1a6df 100644 --- a/packages/commonjs/test/test.js +++ b/packages/commonjs/test/test.js @@ -331,8 +331,8 @@ test('typeof transforms: sinon', async (t) => { } = await bundle.generate({ format: 'es' }); t.is(code.indexOf('typeof require'), -1, code); - // t.not( code.indexOf( 'typeof module' ), -1, code ); // #151 breaks this test - // t.not( code.indexOf( 'typeof define' ), -1, code ); // #144 breaks this test + t.is(code.indexOf('typeof module'), -1, code); + t.is(code.indexOf('typeof define'), -1, code); }); test('deconflicts helper name', async (t) => { @@ -717,3 +717,17 @@ test('throws when there is a dynamic require from outside dynamicRequireRoot', a dynamicRequireRoot }); }); + +test('does not transform typeof exports for mixed modules', async (t) => { + const bundle = await rollup({ + input: 'fixtures/samples/mixed-module-typeof-exports/main.js', + plugins: [commonjs({ transformMixedEsModules: true })] + }); + + const { + output: [{ code }] + } = await bundle.generate({ format: 'es' }); + + t.is(code.includes('typeof exports'), true, '"typeof exports" not found in the code'); + t.snapshot(code); +});