From 9b364ccda41da22d13a2ca8549c11481248e7f8a Mon Sep 17 00:00:00 2001 From: Tomas Chmelevskij Date: Sun, 10 Jan 2021 19:54:28 +0100 Subject: [PATCH 1/3] fix(commonjs): correctly replace shorthand `require` --- packages/commonjs/src/ast-utils.js | 4 ++++ packages/commonjs/src/transform-commonjs.js | 11 ++++++++--- .../test/fixtures/samples/shorthand-require/main.js | 7 +++++++ packages/commonjs/test/test.js | 11 +++++++++++ 4 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 packages/commonjs/test/fixtures/samples/shorthand-require/main.js diff --git a/packages/commonjs/src/ast-utils.js b/packages/commonjs/src/ast-utils.js index a699c57a6..36a42acf5 100644 --- a/packages/commonjs/src/ast-utils.js +++ b/packages/commonjs/src/ast-utils.js @@ -115,3 +115,7 @@ export function isLocallyShadowed(name, scope) { } return false; } + +export function isShorthandProperty(parent) { + return parent && parent.shorthand; +} diff --git a/packages/commonjs/src/transform-commonjs.js b/packages/commonjs/src/transform-commonjs.js index f533b2530..c41937124 100644 --- a/packages/commonjs/src/transform-commonjs.js +++ b/packages/commonjs/src/transform-commonjs.js @@ -11,6 +11,7 @@ import { isDefineCompiledEsm, isFalsy, isReference, + isShorthandProperty, isTruthy, KEY_COMPILED_ESM } from './ast-utils'; @@ -234,10 +235,14 @@ export default function transformCommonjs( )}` ); } + if (isShorthandProperty(parent)) { + magicString.appendRight(node.end, `: ${HELPERS_NAME}.commonjsRequire`); + } else { + magicString.overwrite(node.start, node.end, `${HELPERS_NAME}.commonjsRequire`, { + storeName: true + }); + } - magicString.overwrite(node.start, node.end, `${HELPERS_NAME}.commonjsRequire`, { - storeName: true - }); uses.commonjsHelpers = true; return; case 'module': diff --git a/packages/commonjs/test/fixtures/samples/shorthand-require/main.js b/packages/commonjs/test/fixtures/samples/shorthand-require/main.js new file mode 100644 index 000000000..c42befeb5 --- /dev/null +++ b/packages/commonjs/test/fixtures/samples/shorthand-require/main.js @@ -0,0 +1,7 @@ +const HOST = { + require +}; + +module.exports = { + HOST +}; diff --git a/packages/commonjs/test/test.js b/packages/commonjs/test/test.js index 70a092bc6..32fd5553b 100644 --- a/packages/commonjs/test/test.js +++ b/packages/commonjs/test/test.js @@ -726,6 +726,17 @@ test('does not wrap commonjsRegister calls in createCommonjsModule', async (t) = t.not(/createCommonjsModule\(function/.test(code), true); }); +test('does not replace shorthand `require` property in object', async (t) => { + const bundle = await rollup({ + input: 'fixtures/samples/shorthand-require/main.js', + plugins: [commonjs()] + }); + + const code = await getCodeFromBundle(bundle, { exports: 'named' }); + + t.is(/require: commonjsRequire,/.test(code), true); +}); + // This test uses worker threads to simulate an empty internal cache and needs at least Node 12 if (Number(/^v(\d+)/.exec(process.version)[1]) >= 12) { test('can be cached across instances', async (t) => { From 4b83f0bce9f2fa0ee3d991c2138ef9fe2b5d48e9 Mon Sep 17 00:00:00 2001 From: Tomas Chmelevskij Date: Sun, 10 Jan 2021 20:24:56 +0100 Subject: [PATCH 2/3] fix(commonjs): add comma at the replacement --- packages/commonjs/test/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/commonjs/test/test.js b/packages/commonjs/test/test.js index 32fd5553b..dee5c38b6 100644 --- a/packages/commonjs/test/test.js +++ b/packages/commonjs/test/test.js @@ -734,7 +734,7 @@ test('does not replace shorthand `require` property in object', async (t) => { const code = await getCodeFromBundle(bundle, { exports: 'named' }); - t.is(/require: commonjsRequire,/.test(code), true); + t.is(/require: commonjsRequire/.test(code), true); }); // This test uses worker threads to simulate an empty internal cache and needs at least Node 12 From b771f0245d2d4051f05ff4866dee19098fec7986 Mon Sep 17 00:00:00 2001 From: Tomas Chmelevskij Date: Wed, 20 Jan 2021 20:03:57 +0100 Subject: [PATCH 3/3] feat(commonjs): add parent type check for shorthand util --- packages/commonjs/src/ast-utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/commonjs/src/ast-utils.js b/packages/commonjs/src/ast-utils.js index 36a42acf5..253f20f86 100644 --- a/packages/commonjs/src/ast-utils.js +++ b/packages/commonjs/src/ast-utils.js @@ -117,5 +117,5 @@ export function isLocallyShadowed(name, scope) { } export function isShorthandProperty(parent) { - return parent && parent.shorthand; + return parent && parent.type === 'Property' && parent.shorthand; }