From 0339f9397d63ca625d823ccc31a70ef30df0b48f Mon Sep 17 00:00:00 2001 From: Elarcis Date: Fri, 12 May 2023 16:02:07 +0200 Subject: [PATCH] fix(commonjs)!: dynamic require root check was broken in some cases (#1461) * fix(commonjs): dynamic require root check was broken in some cases * fix(commonjs): fixed a commonjs unit test based on OS paths * test(commonjs): attempt 2 at having tests not fail in CI Using the normalized path in _all_ the properties of the error * test: 1st try at the test * test(commonjs): the dynamic root must ignore conflicting backslashes * test(commonjs): added a test specifically for Windows --- packages/commonjs/src/index.js | 16 ++++--- packages/commonjs/test/helpers/util.js | 5 ++ packages/commonjs/test/test.js | 66 ++++++++++++++++++++++++-- 3 files changed, 76 insertions(+), 11 deletions(-) diff --git a/packages/commonjs/src/index.js b/packages/commonjs/src/index.js index ae0fd9164..dedda0a01 100644 --- a/packages/commonjs/src/index.js +++ b/packages/commonjs/src/index.js @@ -112,6 +112,7 @@ export default function commonjs(options = {}) { let requireResolver; function transformAndCheckExports(code, id) { + const normalizedId = normalizePathSlashes(id); const { isEsModule, hasDefaultExport, hasNamedExports, ast } = analyzeTopLevelStatements( this.parse, code, @@ -127,7 +128,7 @@ export default function commonjs(options = {}) { } if ( - !dynamicRequireModules.has(normalizePathSlashes(id)) && + !dynamicRequireModules.has(normalizedId) && (!(hasCjsKeywords(code, ignoreGlobal) || requireResolver.isRequiredId(id)) || (isEsModule && !options.transformMixedEsModules)) ) { @@ -136,18 +137,19 @@ export default function commonjs(options = {}) { } const needsRequireWrapper = - !isEsModule && - (dynamicRequireModules.has(normalizePathSlashes(id)) || strictRequiresFilter(id)); + !isEsModule && (dynamicRequireModules.has(normalizedId) || strictRequiresFilter(id)); const checkDynamicRequire = (position) => { - if (id.indexOf(dynamicRequireRoot) !== 0) { + const normalizedRequireRoot = normalizePathSlashes(dynamicRequireRoot); + + if (normalizedId.indexOf(normalizedRequireRoot) !== 0) { this.error( { code: 'DYNAMIC_REQUIRE_OUTSIDE_ROOT', - id, + normalizedId, dynamicRequireRoot, - message: `"${id}" contains dynamic require statements but it is not within the current dynamicRequireRoot "${dynamicRequireRoot}". You should set dynamicRequireRoot to "${dirname( - id + message: `"${normalizedId}" contains dynamic require statements but it is not within the current dynamicRequireRoot "${normalizedRequireRoot}". You should set dynamicRequireRoot to "${dirname( + normalizedId )}" or one of its parent directories.` }, position diff --git a/packages/commonjs/test/helpers/util.js b/packages/commonjs/test/helpers/util.js index 060233d71..7ab558988 100644 --- a/packages/commonjs/test/helpers/util.js +++ b/packages/commonjs/test/helpers/util.js @@ -7,6 +7,10 @@ function commonjs(options) { return commonjsPlugin(options); } +function normalizePathSlashes(path) { + return path.replace(/\\/g, '/'); +} + function requireWithContext(code, context) { const module = { exports: {} }; const contextWithExports = { ...context, module, exports: module.exports }; @@ -90,5 +94,6 @@ module.exports = { executeBundle, getCodeFromBundle, getCodeMapFromBundle, + normalizePathSlashes, runCodeSplitTest }; diff --git a/packages/commonjs/test/test.js b/packages/commonjs/test/test.js index 44234545b..948151c0e 100644 --- a/packages/commonjs/test/test.js +++ b/packages/commonjs/test/test.js @@ -13,7 +13,12 @@ const { testBundle } = require('../../../util/test'); const { peerDependencies } = require('../package.json'); -const { commonjs, executeBundle, getCodeFromBundle } = require('./helpers/util.js'); +const { + commonjs, + executeBundle, + getCodeFromBundle, + normalizePathSlashes +} = require('./helpers/util.js'); install(); test.beforeEach(() => process.chdir(__dirname)); @@ -715,9 +720,16 @@ test('throws when there is a dynamic require from outside dynamicRequireRoot', a } 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'); + const id = normalizePathSlashes( + path.join(cwd, 'fixtures/samples/dynamic-require-outside-root/main.js') + ); + const dynamicRequireRoot = normalizePathSlashes( + path.join(cwd, 'fixtures/samples/dynamic-require-outside-root/nested') + ); + const minimalDynamicRequireRoot = normalizePathSlashes( + 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', @@ -726,6 +738,52 @@ test('throws when there is a dynamic require from outside dynamicRequireRoot', a }); }); +test('does not throw when a dynamic require uses different slashes than 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', + dynamicRequireTargets: [ + 'fixtures\\samples\\dynamic-require-outside-root\\nested\\target.js' + ] + }) + ] + }); + } catch (err) { + error = err; + } + + t.is(error, null); +}); + +// On Windows, avoid a false error about a module not being in the dynamic require root due to +// incoherent slashes/backslashes in the paths. +if (os.platform() === 'win32') { + test('correctly asserts dynamicRequireRoot on Windows', 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', + dynamicRequireTargets: [ + 'fixtures/samples/dynamic-require-outside-root/nested/target.js' + ] + }) + ] + }); + } catch (err) { + error = err; + } + + t.is(error, null); + }); +} + test('does not transform typeof exports for mixed modules', async (t) => { const bundle = await rollup({ input: 'fixtures/samples/mixed-module-typeof-exports/main.js',