diff --git a/packages/commonjs/src/dynamic-packages-manager.js b/packages/commonjs/src/dynamic-packages-manager.js index 220e41f8e..3bc559e02 100644 --- a/packages/commonjs/src/dynamic-packages-manager.js +++ b/packages/commonjs/src/dynamic-packages-manager.js @@ -34,26 +34,19 @@ export function getDynamicPackagesModule(dynamicRequireModuleDirPaths, commonDir } export function getDynamicPackagesEntryIntro( - id, dynamicRequireModuleDirPaths, dynamicRequireModuleSet ) { - try { - const code = readFileSync(id, { encoding: 'utf8' }); - let dynamicImports = Array.from( - dynamicRequireModuleSet, - (dynamicId) => `require(${JSON.stringify(DYNAMIC_REGISTER_PREFIX + dynamicId)});` - ).join('\n'); + let dynamicImports = Array.from( + dynamicRequireModuleSet, + (dynamicId) => `require(${JSON.stringify(DYNAMIC_REGISTER_PREFIX + dynamicId)});` + ).join('\n'); - if (dynamicRequireModuleDirPaths.length) { - dynamicImports += `require(${JSON.stringify( - DYNAMIC_REGISTER_PREFIX + DYNAMIC_PACKAGES_ID - )});`; - } - - return `${dynamicImports}\n${code}`; - } catch (ex) { - this.warn(`Failed to read file ${id}, dynamic modules might not work correctly`); - return null; + if (dynamicRequireModuleDirPaths.length) { + dynamicImports += `require(${JSON.stringify( + DYNAMIC_REGISTER_PREFIX + DYNAMIC_PACKAGES_ID + )});`; } + + return dynamicImports; } diff --git a/packages/commonjs/src/index.js b/packages/commonjs/src/index.js index 646da0e18..30bc84126 100644 --- a/packages/commonjs/src/index.js +++ b/packages/commonjs/src/index.js @@ -76,6 +76,13 @@ export default function commonjs(options = {}) { const sourceMap = options.sourceMap !== false; function transformAndCheckExports(code, id) { + if (isDynamicRequireModulesEnabled && this.getModuleInfo(id).isEntry) { + code = getDynamicPackagesEntryIntro( + dynamicRequireModuleDirPaths, + dynamicRequireModuleSet + ) + code; + } + const { isEsModule, hasDefaultExport, hasNamedExports, ast } = checkEsModule( this.parse, code, @@ -176,14 +183,6 @@ export default function commonjs(options = {}) { ); } - if (isDynamicRequireModulesEnabled && this.getModuleInfo(id).isEntry) { - return getDynamicPackagesEntryIntro( - id, - dynamicRequireModuleDirPaths, - dynamicRequireModuleSet - ); - } - return null; }, diff --git a/packages/commonjs/src/transform.js b/packages/commonjs/src/transform.js index db8cc18f4..bf275796d 100644 --- a/packages/commonjs/src/transform.js +++ b/packages/commonjs/src/transform.js @@ -244,7 +244,7 @@ export function transformCommonjs( } if (isDynamicRegister || !isDynamic || sourceId.endsWith('.json')) { - sources.push([sourceId, !isDynamicRegister]); + sources.push([sourceId, isDynamicRegister]); } required[sourceId] = { source: sourceId, name, importsDefault: false, isDynamic }; @@ -545,36 +545,42 @@ export function transformCommonjs( } }); + // If `isEsModule` is on, it means it has ES6 import/export statements, + // which just can't be wrapped in a function. + if (isEsModule) shouldWrap = false; + + usesCommonjsHelpers = usesCommonjsHelpers || shouldWrap; + if ( !sources.length && !uses.module && !uses.exports && !uses.require && + !usesCommonjsHelpers && (ignoreGlobal || !uses.global) ) { // not a CommonJS module return null; } - // If `isEsModule` is on, it means it has ES6 import/export statements, - // which just can't be wrapped in a function. - if (isEsModule) shouldWrap = false; - - usesCommonjsHelpers = usesCommonjsHelpers || shouldWrap; - const importBlock = `${(usesCommonjsHelpers ? [`import * as ${HELPERS_NAME} from '${HELPERS_ID}';`] : [] ) .concat( - sources.map( - ([source]) => - // import the actual module before the proxy, so that we know - // what kind of proxy to build - `import '${source}';` - ), + // dynamic registers first (`commonjsRegister(,,,)`), as the may be required in the other modules + sources + .filter(([, isDynamicRegister]) => isDynamicRegister) + .map(([source]) => `import '${source}';`), + + // now the solid modules, non-commonjsRegister + sources + .filter(([, isDynamicRegister]) => !isDynamicRegister) + .map(([source]) => `import '${source}';`), + + // now the proxies for solid modules (non-commonjsRegister) sources - .filter(([, importProxy]) => importProxy) + .filter(([, isDynamicRegister]) => !isDynamicRegister) .map(([source]) => { const { name, importsDefault } = required[source]; return `import ${importsDefault ? `${name} from ` : ``}'${ diff --git a/packages/commonjs/test/fixtures/function/dynamic-require-instances/package/main.js b/packages/commonjs/test/fixtures/function/dynamic-require-instances/package/main.js index 94793d1e1..1d080f6e9 100755 --- a/packages/commonjs/test/fixtures/function/dynamic-require-instances/package/main.js +++ b/packages/commonjs/test/fixtures/function/dynamic-require-instances/package/main.js @@ -1 +1 @@ -module.exorts = { name: 'package', value: null }; +module.exports = { name: 'package', value: null }; diff --git a/packages/commonjs/test/fixtures/samples/dynamic-require-different-loader/main.js b/packages/commonjs/test/fixtures/samples/dynamic-require-different-loader/main.js new file mode 100755 index 000000000..8adb18ef2 --- /dev/null +++ b/packages/commonjs/test/fixtures/samples/dynamic-require-different-loader/main.js @@ -0,0 +1 @@ +// will be actually be loaded by the custom loader diff --git a/packages/commonjs/test/fixtures/samples/dynamic-require-different-loader/submodule1.js b/packages/commonjs/test/fixtures/samples/dynamic-require-different-loader/submodule1.js new file mode 100755 index 000000000..3156afbff --- /dev/null +++ b/packages/commonjs/test/fixtures/samples/dynamic-require-different-loader/submodule1.js @@ -0,0 +1,2 @@ +const fn = require('./submodule2'); +export default fn; diff --git a/packages/commonjs/test/fixtures/samples/dynamic-require-different-loader/submodule2.js b/packages/commonjs/test/fixtures/samples/dynamic-require-different-loader/submodule2.js new file mode 100755 index 000000000..f470282a9 --- /dev/null +++ b/packages/commonjs/test/fixtures/samples/dynamic-require-different-loader/submodule2.js @@ -0,0 +1,3 @@ +module.exports = function() { + return 'Hello there'; +}; diff --git a/packages/commonjs/test/fixtures/samples/dynamic-require-es-mixed-helpers/importer.js b/packages/commonjs/test/fixtures/samples/dynamic-require-es-mixed-helpers/importer.js new file mode 100755 index 000000000..e1b871582 --- /dev/null +++ b/packages/commonjs/test/fixtures/samples/dynamic-require-es-mixed-helpers/importer.js @@ -0,0 +1 @@ +export default require('./submodule.js'); diff --git a/packages/commonjs/test/fixtures/samples/dynamic-require-es-mixed-helpers/main.js b/packages/commonjs/test/fixtures/samples/dynamic-require-es-mixed-helpers/main.js new file mode 100755 index 000000000..523e6c2dc --- /dev/null +++ b/packages/commonjs/test/fixtures/samples/dynamic-require-es-mixed-helpers/main.js @@ -0,0 +1,5 @@ +/* eslint-disable import/no-dynamic-require, global-require */ + +import result from './importer.js'; + +t.is(result, 'submodule'); diff --git a/packages/commonjs/test/fixtures/samples/dynamic-require-es-mixed-helpers/submodule.js b/packages/commonjs/test/fixtures/samples/dynamic-require-es-mixed-helpers/submodule.js new file mode 100755 index 000000000..c285d34bc --- /dev/null +++ b/packages/commonjs/test/fixtures/samples/dynamic-require-es-mixed-helpers/submodule.js @@ -0,0 +1 @@ +module.exports = 'submodule'; diff --git a/packages/commonjs/test/snapshots/function.js.md b/packages/commonjs/test/snapshots/function.js.md index df9d83977..90242f534 100644 --- a/packages/commonjs/test/snapshots/function.js.md +++ b/packages/commonjs/test/snapshots/function.js.md @@ -1809,7 +1809,7 @@ Generated by [AVA](https://avajs.dev). });␊ ␊ commonjsRegister("/$$rollup_base$$/fixtures/function/dynamic-require-instances/package/main.js", function (module, exports) {␊ - module.exorts = { name: 'package', value: null };␊ + module.exports = { name: 'package', value: null };␊ ␊ });␊ ␊ diff --git a/packages/commonjs/test/snapshots/function.js.snap b/packages/commonjs/test/snapshots/function.js.snap index f159e63c2..ff125fe04 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/test.js b/packages/commonjs/test/test.js index 19cb9ac48..939b25faa 100644 --- a/packages/commonjs/test/test.js +++ b/packages/commonjs/test/test.js @@ -665,3 +665,44 @@ test('imports .cjs file extension by default', async (t) => { const code = await getCodeFromBundle(bundle); t.snapshot(code); }); + +test('registers dynamic requires when entry is from a different loader', async (t) => { + const bundle = await rollup({ + input: 'fixtures/samples/dynamic-require-different-loader/main.js', + plugins: [ + { + load(id) { + if (id === path.resolve('fixtures/samples/dynamic-require-different-loader/main.js')) { + return 'import submodule1 from "./submodule1"; export default submodule1();'; + } + return null; + } + }, + commonjs({ + dynamicRequireTargets: ['fixtures/samples/dynamic-require-different-loader/submodule2.js'], + transformMixedEsModules: true + }) + ] + }); + + t.is((await executeBundle(bundle, t)).exports, 'Hello there'); +}); + +test('transforms the es file with a `commonjsRequire` and no `require`s', async (t) => { + const bundle = await rollup({ + input: 'fixtures/samples/dynamic-require-es-mixed-helpers/main.js', + plugins: [ + commonjs({ + dynamicRequireTargets: ['fixtures/samples/dynamic-require-es-mixed-helpers/submodule.js'], + transformMixedEsModules: true + }) + ] + }); + + const code = await getCodeFromBundle(bundle); + + t.is( + /commonjsRequire\(["']\.\/submodule\.js/.test(code), + true + ); +});