Skip to content

Commit

Permalink
feat(commonjs): throw for dynamic requires from outside the configure…
Browse files Browse the repository at this point in the history
…d root (#1038)
  • Loading branch information
lukastaegert committed Apr 24, 2022
1 parent 08ab82c commit c198d9d
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 15 deletions.
18 changes: 16 additions & 2 deletions packages/commonjs/src/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { extname, relative, resolve } from 'path';
import { extname, relative, resolve, dirname } from 'path';

import { createFilter } from '@rollup/pluginutils';

Expand Down Expand Up @@ -129,6 +129,19 @@ export default function commonjs(options = {}) {
!isEsModule &&
(dynamicRequireModules.has(normalizePathSlashes(id)) || strictRequiresFilter(id));

const checkDynamicRequire = () => {
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.`
});
}
};

return transformCommonjs(
this.parse,
code,
Expand All @@ -146,7 +159,8 @@ export default function commonjs(options = {}) {
getDefaultIsModuleExports(id),
needsRequireWrapper,
resolveRequireSourcesAndGetMeta(this),
isRequiredId(id)
isRequiredId(id),
checkDynamicRequire
);
}

Expand Down
4 changes: 3 additions & 1 deletion packages/commonjs/src/transform-commonjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ export default async function transformCommonjs(
defaultIsModuleExports,
needsRequireWrapper,
resolveRequireSourcesAndGetMeta,
isRequired
isRequired,
checkDynamicRequire
) {
const ast = astCache || tryParse(parse, code, id);
const magicString = new MagicString(code);
Expand Down Expand Up @@ -295,6 +296,7 @@ export default async function transformCommonjs(
);
}
if (!ignoreDynamicRequires) {
checkDynamicRequire();
if (isShorthandProperty(parent)) {
magicString.appendRight(node.end, `: ${dynamicRequireName}`);
} else {
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
function takeModule(path) {
// eslint-disable-next-line global-require,import/no-dynamic-require
return require(path);
}

takeModule('./nested/target.js');
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = 'target';
28 changes: 28 additions & 0 deletions packages/commonjs/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -684,3 +684,31 @@ if (os.platform() === 'win32') {
t.regex(code, /var foo(\$\d+)? = {}/);
});
}

test('throws when there is a dynamic require from outside dynamicRequireRoot', async (t) => {
let error = null;
try {
await rollup({
input: 'fixtures/samples/dynamic-require-outside-root/main.js',
plugins: [
commonjs({
dynamicRequireRoot: 'fixtures/samples/dynamic-require-outside-root/nested',
dynamicRequireTargets: ['fixtures/samples/dynamic-require-outside-root/nested/target.js']
})
]
});
} catch (err) {
error = err;
}

const cwd = process.cwd();
const id = path.join(cwd, 'fixtures/samples/dynamic-require-outside-root/main.js');
const dynamicRequireRoot = path.join(cwd, 'fixtures/samples/dynamic-require-outside-root/nested');
const minimalDynamicRequireRoot = path.join(cwd, 'fixtures/samples/dynamic-require-outside-root');
t.like(error, {
message: `"${id}" contains dynamic require statements but it is not within the current dynamicRequireRoot "${dynamicRequireRoot}". You should set dynamicRequireRoot to "${minimalDynamicRequireRoot}" or one of its parent directories.`,
pluginCode: 'DYNAMIC_REQUIRE_OUTSIDE_ROOT',
id,
dynamicRequireRoot
});
});

0 comments on commit c198d9d

Please sign in to comment.