From a684430c9be7ba31d5b2e6e440b5aac916d93b68 Mon Sep 17 00:00:00 2001 From: gagdiez Date: Sat, 4 Nov 2023 17:02:24 +0100 Subject: [PATCH 1/4] fix: hasOwnProperty The current implementation makes a direct use of hasOwnProperty which misbehaves when a module exports a method named `hasOwnProperty` [see issue 9350](https://github.com/parcel-bundler/parcel/issues/9350). This commit implements a [safer way to call hasOwnProperty](https://stackoverflow.com/questions/12017693/why-use-object-prototype-hasownproperty-callmyobj-prop-instead-of-myobj-hasow/12017703#12017703). --- packages/transformers/js/src/esmodule-helpers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/transformers/js/src/esmodule-helpers.js b/packages/transformers/js/src/esmodule-helpers.js index 21b672e3afb..0018c1ef8a3 100644 --- a/packages/transformers/js/src/esmodule-helpers.js +++ b/packages/transformers/js/src/esmodule-helpers.js @@ -8,7 +8,7 @@ exports.defineInteropFlag = function (a) { exports.exportAll = function (source, dest) { Object.keys(source).forEach(function (key) { - if (key === 'default' || key === '__esModule' || dest.hasOwnProperty(key)) { + if (key === 'default' || key === '__esModule' || Object.prototype.hasOwnProperty.call(dest, key)) { return; } From c3bd2a23c753dc229d8f0366f704dc17650f8f0b Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Sun, 12 Nov 2023 16:24:15 +0100 Subject: [PATCH 2/4] Fix for scope-hoisting --- packages/packagers/js/src/helpers.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/packagers/js/src/helpers.js b/packages/packagers/js/src/helpers.js index f91b86d5ac7..de455c32c57 100644 --- a/packages/packagers/js/src/helpers.js +++ b/packages/packagers/js/src/helpers.js @@ -112,7 +112,7 @@ function $parcel$export(e, n, v, s) { const $parcel$exportWildcard = ` function $parcel$exportWildcard(dest, source) { Object.keys(source).forEach(function(key) { - if (key === 'default' || key === '__esModule' || dest.hasOwnProperty(key)) { + if (key === 'default' || key === '__esModule' || Object.prototype.hasOwnProperty.call(dest, key)) { return; } From 59bf2498f4d1c9beba647c3f17489bbec36a534e Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Sun, 12 Nov 2023 16:24:20 +0100 Subject: [PATCH 3/4] Add test --- .../core/integration-tests/test/javascript.js | 113 +++++++++++------- 1 file changed, 73 insertions(+), 40 deletions(-) diff --git a/packages/core/integration-tests/test/javascript.js b/packages/core/integration-tests/test/javascript.js index fb377b85990..b20248f9a2f 100644 --- a/packages/core/integration-tests/test/javascript.js +++ b/packages/core/integration-tests/test/javascript.js @@ -6386,6 +6386,48 @@ describe('javascript', function () { assert(!contents.includes('\ufffe')); }); + it(`should not wrap assets that are duplicated in different targets`, async function () { + const dir = path.join(__dirname, 'multi-target-duplicates'); + overlayFS.mkdirp(dir); + + await fsFixture(overlayFS, dir)` + shared/index.js: + export default 2; + + packages/a/package.json: + { + "source": "index.js", + "module": "dist/module.js" + } + + packages/a/index.js: + import shared from '../../shared'; + export default shared + 2; + + packages/b/package.json: + { + "source": "index.js", + "module": "dist/module.js" + } + + packages/b/index.js: + import shared from '../../shared'; + export default shared + 2; + `; + + let b = await bundle(path.join(dir, '/packages/*'), { + inputFS: overlayFS, + }); + + for (let bundle of b.getBundles()) { + let contents = await outputFS.readFile(bundle.filePath, 'utf8'); + assert( + !contents.includes('parcelRequire'), + 'should not include parcelRequire', + ); + } + }); + for (let shouldScopeHoist of [false, true]) { let options = { defaultTargetOptions: { @@ -7532,7 +7574,9 @@ describe('javascript', function () { assert.equal(res.output, 123); }); - it('duplicate assets should share module scope', async function () { + it(`duplicate assets should share module scope ${ + shouldScopeHoist ? 'with' : 'without' + } scope-hoisting`, async function () { let b = await bundle( [ path.join( @@ -7552,46 +7596,35 @@ describe('javascript', function () { assert.equal(await result.output, 2); }); - it('should not wrap assets that are duplicated in different targets', async function () { - const dir = path.join(__dirname, 'multi-target-duplicates'); - overlayFS.mkdirp(dir); - - await fsFixture(overlayFS, dir)` - shared/index.js: - export default 2; - - packages/a/package.json: - { - "source": "index.js", - "module": "dist/module.js" - } - - packages/a/index.js: - import shared from '../../shared'; - export default shared + 2; - - packages/b/package.json: - { - "source": "index.js", - "module": "dist/module.js" - } - - packages/b/index.js: - import shared from '../../shared'; - export default shared + 2; - `; - - let b = await bundle(path.join(dir, '/packages/*'), { - inputFS: overlayFS, - }); + it(`should work correctly with export called hasOwnProperty ${ + shouldScopeHoist ? 'with' : 'without' + } scope-hoisting`, async () => { + await fsFixture(overlayFS, __dirname)` + js-export-all-hasOwnProperty + a.js: + export function hasOwnProperty() { + throw new Error("Shouldn't be called"); + } + b.js: + module.exports = { other: 123 }; + + library.js: + export * from './a'; + export * from './b'; + + index.js: + import * as x from './library'; + output = sideEffectNoop(x).other;`; - for (let bundle of b.getBundles()) { - let contents = await outputFS.readFile(bundle.filePath, 'utf8'); - assert( - !contents.includes('parcelRequire'), - 'should not include parcelRequire', - ); - } + let b = await bundle( + path.join(__dirname, 'js-export-all-hasOwnProperty/index.js'), + { + ...options, + inputFS: overlayFS, + }, + ); + let res = await run(b, null, {require: false}); + assert.equal(res.output, 123); }); } }); From cefec13d8086398f5963ec519299eb3bc21fc6b9 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Sun, 12 Nov 2023 16:46:39 +0100 Subject: [PATCH 4/4] Lint --- packages/transformers/js/src/esmodule-helpers.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/transformers/js/src/esmodule-helpers.js b/packages/transformers/js/src/esmodule-helpers.js index 0018c1ef8a3..8b343f2aa96 100644 --- a/packages/transformers/js/src/esmodule-helpers.js +++ b/packages/transformers/js/src/esmodule-helpers.js @@ -8,7 +8,11 @@ exports.defineInteropFlag = function (a) { exports.exportAll = function (source, dest) { Object.keys(source).forEach(function (key) { - if (key === 'default' || key === '__esModule' || Object.prototype.hasOwnProperty.call(dest, key)) { + if ( + key === 'default' || + key === '__esModule' || + Object.prototype.hasOwnProperty.call(dest, key) + ) { return; }