diff --git a/packages/commonjs/README.md b/packages/commonjs/README.md index 8ef7f2a0c..724ca95b0 100644 --- a/packages/commonjs/README.md +++ b/packages/commonjs/README.md @@ -42,6 +42,8 @@ export default { Then call `rollup` either via the [CLI](https://www.rollupjs.org/guide/en/#command-line-reference) or the [API](https://www.rollupjs.org/guide/en/#javascript-api). +When used together with the node-resolve plugin + ## Options ### `strictRequires` @@ -378,10 +380,12 @@ export default { format: 'iife', name: 'MyModule' }, - plugins: [resolve(), commonjs()] + plugins: [commonjs(), resolve()] }; ``` +Note that this plugin needs to be placed _before_ the node-resolve plugin. If that is not the case, it will automatically fix this by adjusting the plugins array and moving the node-resolve plugin directly behind the commonjs plugin during startup. + ## Usage with symlinks Symlinks are common in monorepos and are also created by the `npm link` command. Rollup with `@rollup/plugin-node-resolve` resolves modules to their real paths by default. So `include` and `exclude` paths should handle real paths rather than symlinked paths (e.g. `../common/node_modules/**` instead of `node_modules/**`). You may also use a regular expression for `include` that works regardless of base path. Try this: diff --git a/packages/commonjs/src/dynamic-modules.js b/packages/commonjs/src/dynamic-modules.js index 02b9845f3..eb4a8a3f2 100644 --- a/packages/commonjs/src/dynamic-modules.js +++ b/packages/commonjs/src/dynamic-modules.js @@ -1,10 +1,63 @@ +import { existsSync, readFileSync, statSync } from 'fs'; +import { join, resolve } from 'path'; + +import glob from 'glob'; + import { getVirtualPathForDynamicRequirePath, normalizePathSlashes } from './utils'; +function getPackageEntryPoint(dirPath) { + let entryPoint = 'index.js'; + + try { + if (existsSync(join(dirPath, 'package.json'))) { + entryPoint = + JSON.parse(readFileSync(join(dirPath, 'package.json'), { encoding: 'utf8' })).main || + entryPoint; + } + } catch (ignored) { + // ignored + } + + return entryPoint; +} + +function isDirectory(path) { + try { + if (statSync(path).isDirectory()) return true; + } catch (ignored) { + // Nothing to do here + } + return false; +} + +export function getDynamicRequireModules(patterns) { + const dynamicRequireModules = new Map(); + for (const pattern of !patterns || Array.isArray(patterns) ? patterns || [] : [patterns]) { + const isNegated = pattern.startsWith('!'); + const modifyMap = (targetPath, resolvedPath) => + isNegated + ? dynamicRequireModules.delete(targetPath) + : dynamicRequireModules.set(targetPath, resolvedPath); + for (const path of glob.sync(isNegated ? pattern.substr(1) : pattern)) { + const resolvedPath = resolve(path); + const requirePath = normalizePathSlashes(resolvedPath); + if (isDirectory(resolvedPath)) { + const modulePath = normalizePathSlashes(resolve(join(path, getPackageEntryPoint(path)))); + modifyMap(requirePath, modulePath); + modifyMap(modulePath, modulePath); + } else { + modifyMap(requirePath, requirePath); + } + } + } + return dynamicRequireModules; +} + const FAILED_REQUIRE_ERROR = `throw new Error('Could not dynamically require "' + path + '". Please configure the dynamicRequireTargets or/and ignoreDynamicRequires option of @rollup/plugin-commonjs appropriately for this require call to work.');`; -export function getDynamicRequireModules( +export function getDynamicModuleRegistry( isDynamicRequireModulesEnabled, - dynamicRequireModuleSet, + dynamicRequireModules, commonDir, ignoreDynamicRequires ) { @@ -13,8 +66,7 @@ export function getDynamicRequireModules( ${FAILED_REQUIRE_ERROR} }`; } - const dynamicModuleIds = [...dynamicRequireModuleSet]; - const dynamicModuleImports = dynamicModuleIds + const dynamicModuleImports = [...dynamicRequireModules.values()] .map( (id, index) => `import ${ @@ -22,7 +74,7 @@ export function getDynamicRequireModules( } from ${JSON.stringify(id)};` ) .join('\n'); - const dynamicModuleProps = dynamicModuleIds + const dynamicModuleProps = [...dynamicRequireModules.keys()] .map( (id, index) => `\t\t${JSON.stringify( diff --git a/packages/commonjs/src/dynamic-require-paths.js b/packages/commonjs/src/dynamic-require-paths.js deleted file mode 100644 index 849ab1a9e..000000000 --- a/packages/commonjs/src/dynamic-require-paths.js +++ /dev/null @@ -1,46 +0,0 @@ -import { existsSync, readFileSync, statSync } from 'fs'; -import { join, resolve } from 'path'; - -import glob from 'glob'; - -import { normalizePathSlashes } from './utils'; - -function getPackageEntryPoint(dirPath) { - let entryPoint = 'index.js'; - - try { - if (existsSync(join(dirPath, 'package.json'))) { - entryPoint = - JSON.parse(readFileSync(join(dirPath, 'package.json'), { encoding: 'utf8' })).main || - entryPoint; - } - } catch (ignored) { - // ignored - } - - return entryPoint; -} - -function isDirectory(path) { - try { - if (statSync(path).isDirectory()) return true; - } catch (ignored) { - // Nothing to do here - } - return false; -} - -export default function getDynamicRequireModuleSet(patterns) { - const dynamicRequireModuleSet = new Set(); - for (const pattern of !patterns || Array.isArray(patterns) ? patterns || [] : [patterns]) { - const isNegated = pattern.startsWith('!'); - const modifySet = Set.prototype[isNegated ? 'delete' : 'add'].bind(dynamicRequireModuleSet); - for (const path of glob.sync(isNegated ? pattern.substr(1) : pattern)) { - modifySet(normalizePathSlashes(resolve(path))); - if (isDirectory(path)) { - modifySet(normalizePathSlashes(resolve(join(path, getPackageEntryPoint(path))))); - } - } - } - return dynamicRequireModuleSet; -} diff --git a/packages/commonjs/src/generate-imports.js b/packages/commonjs/src/generate-imports.js index e90152d36..f3c976a63 100644 --- a/packages/commonjs/src/generate-imports.js +++ b/packages/commonjs/src/generate-imports.js @@ -68,11 +68,11 @@ export function getRequireStringArg(node) { : node.arguments[0].quasis[0].value.cooked; } -export function hasDynamicModuleForPath(source, id, dynamicRequireModuleSet) { +export function hasDynamicModuleForPath(source, id, dynamicRequireModules) { if (!/^(?:\.{0,2}[/\\]|[A-Za-z]:[/\\])/.test(source)) { try { const resolvedPath = normalizePathSlashes(nodeResolveSync(source, { basedir: dirname(id) })); - if (dynamicRequireModuleSet.has(resolvedPath)) { + if (dynamicRequireModules.has(resolvedPath)) { return true; } } catch (ex) { @@ -85,7 +85,7 @@ export function hasDynamicModuleForPath(source, id, dynamicRequireModuleSet) { for (const attemptExt of ['', '.js', '.json']) { const resolvedPath = normalizePathSlashes(resolve(dirname(id), source + attemptExt)); - if (dynamicRequireModuleSet.has(resolvedPath)) { + if (dynamicRequireModules.has(resolvedPath)) { return true; } } diff --git a/packages/commonjs/src/index.js b/packages/commonjs/src/index.js index 892a4eaf3..83856200a 100644 --- a/packages/commonjs/src/index.js +++ b/packages/commonjs/src/index.js @@ -6,9 +6,8 @@ import getCommonDir from 'commondir'; import { peerDependencies } from '../package.json'; import analyzeTopLevelStatements from './analyze-top-level-statements'; -import { getDynamicRequireModules } from './dynamic-modules'; +import { getDynamicModuleRegistry, getDynamicRequireModules } from './dynamic-modules'; -import getDynamicRequireModuleSet from './dynamic-require-paths'; import { DYNAMIC_MODULES_ID, ES_IMPORT_SUFFIX, @@ -61,10 +60,11 @@ export default function commonjs(options = {}) { getWrappedIds, isRequiredId } = getResolveRequireSourcesAndGetMeta(extensions, detectCycles); - const dynamicRequireModuleSet = getDynamicRequireModuleSet(options.dynamicRequireTargets); - const isDynamicRequireModulesEnabled = dynamicRequireModuleSet.size > 0; + const dynamicRequireModules = getDynamicRequireModules(options.dynamicRequireTargets); + const isDynamicRequireModulesEnabled = dynamicRequireModules.size > 0; + // TODO Lukas do we need the CWD? const commonDir = isDynamicRequireModulesEnabled - ? getCommonDir(null, Array.from(dynamicRequireModuleSet).concat(process.cwd())) + ? getCommonDir(null, Array.from(dynamicRequireModules.keys()).concat(process.cwd())) : null; const esModulesWithDefaultExport = new Set(); @@ -111,7 +111,7 @@ export default function commonjs(options = {}) { } if ( - !dynamicRequireModuleSet.has(normalizePathSlashes(id)) && + !dynamicRequireModules.has(normalizePathSlashes(id)) && (!(hasCjsKeywords(code, ignoreGlobal) || isRequiredId(id)) || (isEsModule && !options.transformMixedEsModules)) ) { @@ -120,7 +120,7 @@ export default function commonjs(options = {}) { const needsRequireWrapper = !isEsModule && - (dynamicRequireModuleSet.has(normalizePathSlashes(id)) || strictRequiresFilter(id)); + (dynamicRequireModules.has(normalizePathSlashes(id)) || strictRequiresFilter(id)); return transformCommonjs( this.parse, @@ -133,7 +133,7 @@ export default function commonjs(options = {}) { getIgnoreTryCatchRequireStatementMode, sourceMap, isDynamicRequireModulesEnabled, - dynamicRequireModuleSet, + dynamicRequireModules, commonDir, ast, defaultIsModuleExports, @@ -146,10 +146,9 @@ export default function commonjs(options = {}) { return { name: 'commonjs', - options(options) { + options({ plugins }) { // Always sort the node-resolve plugin after the commonjs plugin as otherwise CommonJS entries // will not work with strictRequires: true - const { plugins } = options; if (Array.isArray(plugins)) { const cjsIndex = plugins.findIndex((plugin) => plugin.name === 'commonjs'); const nodeResolveIndex = plugins.findIndex((plugin) => plugin.name === 'node-resolve'); @@ -228,9 +227,9 @@ export default function commonjs(options = {}) { } if (id === DYNAMIC_MODULES_ID) { - return getDynamicRequireModules( + return getDynamicModuleRegistry( isDynamicRequireModulesEnabled, - dynamicRequireModuleSet, + dynamicRequireModules, commonDir, ignoreDynamicRequires ); diff --git a/packages/commonjs/src/transform-commonjs.js b/packages/commonjs/src/transform-commonjs.js index c6cfbdc1a..f0c43c362 100644 --- a/packages/commonjs/src/transform-commonjs.js +++ b/packages/commonjs/src/transform-commonjs.js @@ -46,7 +46,7 @@ export default async function transformCommonjs( getIgnoreTryCatchRequireStatementMode, sourceMap, isDynamicRequireModulesEnabled, - dynamicRequireModuleSet, + dynamicRequireModules, commonDir, astCache, defaultIsModuleExports, @@ -195,7 +195,7 @@ export default async function transformCommonjs( node.callee.object && node.callee.object.name === 'require' && node.callee.property.name === 'resolve' && - hasDynamicModuleForPath(id, '/', dynamicRequireModuleSet) + hasDynamicModuleForPath(id, '/', dynamicRequireModules) ) { // TODO Lukas reimplement? uses.require = true; @@ -288,7 +288,7 @@ export default async function transformCommonjs( uses.require = true; if (isNodeRequirePropertyAccess(parent)) { // TODO Lukas reimplement? - if (hasDynamicModuleForPath(id, '/', dynamicRequireModuleSet)) { + if (hasDynamicModuleForPath(id, '/', dynamicRequireModules)) { if (parent.property.name === 'cache') { magicString.overwrite(node.start, node.end, dynamicRequireName, { storeName: true diff --git a/packages/commonjs/test/snapshots/function.js.md b/packages/commonjs/test/snapshots/function.js.md index 260d58e48..6e8024273 100644 --- a/packages/commonjs/test/snapshots/function.js.md +++ b/packages/commonjs/test/snapshots/function.js.md @@ -5624,7 +5624,7 @@ Generated by [AVA](https://avajs.dev). `, } -## strict-requires-entry +## strict-requires-entry-node-resolve > Snapshot 1 @@ -5883,40 +5883,6 @@ Generated by [AVA](https://avajs.dev). `, } -## strict-requires-magic-string - -> Snapshot 1 - - { - 'main.js': `'use strict';␊ - ␊ - Object.defineProperty(exports, '__esModule', { value: true });␊ - ␊ - var main = {};␊ - ␊ - var hasRequiredMain;␊ - ␊ - function requireMain () {␊ - if (hasRequiredMain) return main;␊ - hasRequiredMain = 1;␊ - console.log('hey');␊ - // magic-string@0.25.7␊ - const m = new MagicString('0123456789');␊ - console.log(␊ - m.prependRight(0, 'W').prependLeft(3, 'AB').appendRight(9, 'XY').remove(6, 8).toString()␊ - );␊ - const bundle = new MagicString.Bundle();␊ - bundle.addSource({ filename: 'foo.txt', content: m });␊ - const map = bundle.generateMap({ file: 'bundle.txt', includeContent: true, hires: true });␊ - console.log(JSON.stringify(map));␊ - main.foo = 'foo';␊ - return main;␊ - }␊ - ␊ - exports.__require = requireMain;␊ - `, - } - ## strict-requires-multiple-entry > Snapshot 1 @@ -6807,27 +6773,3 @@ Generated by [AVA](https://avajs.dev). module.exports = main;␊ `, } - -## strict-requires-entry-node-resolve - -> Snapshot 1 - - { - 'main.js': `'use strict';␊ - ␊ - var main = {};␊ - ␊ - var hasRequiredMain;␊ - ␊ - function requireMain () {␊ - if (hasRequiredMain) return main;␊ - hasRequiredMain = 1;␊ - main.foo = 'foo';␊ - return main;␊ - }␊ - ␊ - var mainExports = requireMain();␊ - ␊ - module.exports = mainExports;␊ - `, - } diff --git a/packages/commonjs/test/snapshots/function.js.snap b/packages/commonjs/test/snapshots/function.js.snap index 4ec48d232..285f92e78 100644 Binary files a/packages/commonjs/test/snapshots/function.js.snap and b/packages/commonjs/test/snapshots/function.js.snap differ