From 5a5ce1fc289daf23919a397ae7ca498b5bd8eae8 Mon Sep 17 00:00:00 2001 From: the_mcmurder <3059715+TheMcMurder@users.noreply.github.com> Date: Tue, 22 Mar 2022 08:31:43 -0600 Subject: [PATCH 1/5] feat(node-resolve): deep side effects option --- packages/node-resolve/README.md | 7 ++++ packages/node-resolve/src/index.js | 8 +++-- .../src/resolveImportSpecifiers.js | 29 +++++++++------ packages/node-resolve/src/util.js | 9 +++-- .../deep-side-effects/deep/side-effect.js | 1 + .../test/fixtures/deep-side-effects/index.js | 4 +++ .../fixtures/deep-side-effects/package.json | 4 +++ .../deep-side-effects/shallow-side-effect.js | 1 + .../node-resolve/test/snapshots/test.js.md | 24 +++++++++++++ .../node-resolve/test/snapshots/test.js.snap | Bin 1459 -> 1568 bytes packages/node-resolve/test/test.js | 33 ++++++++++++++++++ 11 files changed, 105 insertions(+), 15 deletions(-) create mode 100644 packages/node-resolve/test/fixtures/deep-side-effects/deep/side-effect.js create mode 100644 packages/node-resolve/test/fixtures/deep-side-effects/index.js create mode 100644 packages/node-resolve/test/fixtures/deep-side-effects/package.json create mode 100644 packages/node-resolve/test/fixtures/deep-side-effects/shallow-side-effect.js diff --git a/packages/node-resolve/README.md b/packages/node-resolve/README.md index 905d15046..e5e73552b 100755 --- a/packages/node-resolve/README.md +++ b/packages/node-resolve/README.md @@ -163,6 +163,13 @@ rootDir: path.join(process.cwd(), '..') If you use the `sideEffects` property in the package.json, by default this is respected for files in the root package. Set to `true` to ignore the `sideEffects` configuration for the root package. +### `deepSideEffects` + +Type: `Boolean`
+Default: `false` + +A boolean which instructs how to handle deep side effects. This is useful for packages that have nested files in their dist directory and also utilize `sideEffects` configuration in their package.json. + ## Preserving symlinks This plugin honours the rollup [`preserveSymlinks`](https://rollupjs.org/guide/en/#preservesymlinks) option. diff --git a/packages/node-resolve/src/index.js b/packages/node-resolve/src/index.js index 3eab84bfe..862a07875 100644 --- a/packages/node-resolve/src/index.js +++ b/packages/node-resolve/src/index.js @@ -37,7 +37,8 @@ const defaults = { extensions: ['.mjs', '.js', '.json', '.node'], resolveOnly: [], moduleDirectories: ['node_modules'], - ignoreSideEffectsForRoot: false + ignoreSideEffectsForRoot: false, + deepSideEffects: false, }; export const DEFAULTS = deepFreeze(deepMerge({}, defaults)); @@ -45,7 +46,7 @@ export function nodeResolve(opts = {}) { const { warnings } = handleDeprecatedOptions(opts); const options = { ...defaults, ...opts }; - const { extensions, jail, moduleDirectories, ignoreSideEffectsForRoot } = options; + const { extensions, jail, moduleDirectories, ignoreSideEffectsForRoot, deepSideEffects } = options; const conditionsEsm = [...baseConditionsEsm, ...(options.exportConditions || [])]; const conditionsCjs = [...baseConditionsCjs, ...(options.exportConditions || [])]; const packageInfoCache = new Map(); @@ -163,7 +164,8 @@ export function nodeResolve(opts = {}) { baseDir, moduleDirectories, rootDir, - ignoreSideEffectsForRoot + ignoreSideEffectsForRoot, + deepSideEffects }); const importeeIsBuiltin = builtins.has(importee); diff --git a/packages/node-resolve/src/resolveImportSpecifiers.js b/packages/node-resolve/src/resolveImportSpecifiers.js index e0a5b0466..bcf718edf 100644 --- a/packages/node-resolve/src/resolveImportSpecifiers.js +++ b/packages/node-resolve/src/resolveImportSpecifiers.js @@ -44,7 +44,8 @@ async function resolveIdClassic({ baseDir, moduleDirectories, rootDir, - ignoreSideEffectsForRoot + ignoreSideEffectsForRoot, + deepSideEffects }) { let hasModuleSideEffects = () => null; let hasPackageEntry = true; @@ -61,7 +62,8 @@ async function resolveIdClassic({ preserveSymlinks, useBrowserOverrides, rootDir, - ignoreSideEffectsForRoot + ignoreSideEffectsForRoot, + deepSideEffects }); ({ packageInfo, hasModuleSideEffects, hasPackageEntry, packageBrowserField } = info); @@ -112,7 +114,8 @@ async function resolveWithExportMap({ baseDir, moduleDirectories, rootDir, - ignoreSideEffectsForRoot + ignoreSideEffectsForRoot, + deepSideEffects, }) { if (importSpecifier.startsWith('#')) { // this is a package internal import, resolve using package imports field @@ -130,7 +133,7 @@ async function resolveWithExportMap({ preserveSymlinks, useBrowserOverrides, baseDir, - moduleDirectories + moduleDirectories, }); } }); @@ -164,7 +167,8 @@ async function resolveWithExportMap({ preserveSymlinks, useBrowserOverrides, rootDir, - ignoreSideEffectsForRoot + ignoreSideEffectsForRoot, + deepSideEffects, }); ({ packageInfo, hasModuleSideEffects, hasPackageEntry, packageBrowserField } = info); @@ -231,7 +235,8 @@ async function resolveWithClassic({ baseDir, moduleDirectories, rootDir, - ignoreSideEffectsForRoot + ignoreSideEffectsForRoot, + deepSideEffects, }) { for (let i = 0; i < importSpecifierList.length; i++) { // eslint-disable-next-line no-await-in-loop @@ -248,7 +253,8 @@ async function resolveWithClassic({ baseDir, moduleDirectories, rootDir, - ignoreSideEffectsForRoot + ignoreSideEffectsForRoot, + deepSideEffects, }); if (result) { @@ -276,7 +282,8 @@ export default async function resolveImportSpecifiers({ baseDir, moduleDirectories, rootDir, - ignoreSideEffectsForRoot + ignoreSideEffectsForRoot, + deepSideEffects, }) { try { const exportMapRes = await resolveWithExportMap({ @@ -291,7 +298,8 @@ export default async function resolveImportSpecifiers({ baseDir, moduleDirectories, rootDir, - ignoreSideEffectsForRoot + ignoreSideEffectsForRoot, + deepSideEffects, }); if (exportMapRes) return exportMapRes; } catch (error) { @@ -316,6 +324,7 @@ export default async function resolveImportSpecifiers({ baseDir, moduleDirectories, rootDir, - ignoreSideEffectsForRoot + ignoreSideEffectsForRoot, + deepSideEffects, }); } diff --git a/packages/node-resolve/src/util.js b/packages/node-resolve/src/util.js index 7b21d250a..07373e6e4 100644 --- a/packages/node-resolve/src/util.js +++ b/packages/node-resolve/src/util.js @@ -48,7 +48,8 @@ export function getPackageInfo(options) { preserveSymlinks, useBrowserOverrides, rootDir, - ignoreSideEffectsForRoot + ignoreSideEffectsForRoot, + deepSideEffects, } = options; let { pkgPath } = options; @@ -144,7 +145,11 @@ export function getPackageInfo(options) { if (typeof packageSideEffects === 'boolean') { internalPackageInfo.hasModuleSideEffects = () => packageSideEffects; } else if (Array.isArray(packageSideEffects)) { - internalPackageInfo.hasModuleSideEffects = createFilter(packageSideEffects, null, { + let _packageSideEffects = packageSideEffects + if (deepSideEffects) { + _packageSideEffects = _packageSideEffects.map((sideEffect) => `**/${sideEffect}`) + } + internalPackageInfo.hasModuleSideEffects = createFilter(_packageSideEffects, null, { resolve: pkgRoot }); } diff --git a/packages/node-resolve/test/fixtures/deep-side-effects/deep/side-effect.js b/packages/node-resolve/test/fixtures/deep-side-effects/deep/side-effect.js new file mode 100644 index 000000000..a9932c054 --- /dev/null +++ b/packages/node-resolve/test/fixtures/deep-side-effects/deep/side-effect.js @@ -0,0 +1 @@ +console.log('deep side effect') \ No newline at end of file diff --git a/packages/node-resolve/test/fixtures/deep-side-effects/index.js b/packages/node-resolve/test/fixtures/deep-side-effects/index.js new file mode 100644 index 000000000..2b08cba9b --- /dev/null +++ b/packages/node-resolve/test/fixtures/deep-side-effects/index.js @@ -0,0 +1,4 @@ +import './deep/side-effect.js' +import './shallow-side-effect.js' + +console.log('main') \ No newline at end of file diff --git a/packages/node-resolve/test/fixtures/deep-side-effects/package.json b/packages/node-resolve/test/fixtures/deep-side-effects/package.json new file mode 100644 index 000000000..cef46cb21 --- /dev/null +++ b/packages/node-resolve/test/fixtures/deep-side-effects/package.json @@ -0,0 +1,4 @@ +{ + "main": "./index.js", + "sideEffects": ["*.js"] +} diff --git a/packages/node-resolve/test/fixtures/deep-side-effects/shallow-side-effect.js b/packages/node-resolve/test/fixtures/deep-side-effects/shallow-side-effect.js new file mode 100644 index 000000000..dd89c4709 --- /dev/null +++ b/packages/node-resolve/test/fixtures/deep-side-effects/shallow-side-effect.js @@ -0,0 +1 @@ +console.log('shallow side effect') \ No newline at end of file diff --git a/packages/node-resolve/test/snapshots/test.js.md b/packages/node-resolve/test/snapshots/test.js.md index 844b66a7c..77aae2a80 100644 --- a/packages/node-resolve/test/snapshots/test.js.md +++ b/packages/node-resolve/test/snapshots/test.js.md @@ -176,3 +176,27 @@ Generated by [AVA](https://avajs.dev). Error { message: 'node-resolve: `customResolveOptions.packageIterator` is no longer an option. If you need this, please open an issue.', } + +## respects the package.json sideEffects property for files in the root package and does not do deep side effects by default + +> Snapshot 1 + + `'use strict';␊ + ␊ + console.log('shallow side effect');␊ + ␊ + console.log('main');␊ + ` + +## respects the package.json sideEffects property for files in the root package and supports deep side effects + +> Snapshot 1 + + `'use strict';␊ + ␊ + console.log('deep side effect');␊ + ␊ + console.log('shallow side effect');␊ + ␊ + console.log('main');␊ + ` diff --git a/packages/node-resolve/test/snapshots/test.js.snap b/packages/node-resolve/test/snapshots/test.js.snap index 956e33b1a44fa8907def64051a9db40f8b7a0588..b5e0acb408a3e76151250697b72f215f2c6f9177 100644 GIT binary patch literal 1568 zcmV+*2H*KXRzVZe+&@e4-ER*Z}R1y^ZU;0 z`@Y|I&R1$eCEG8N*1Xd%SkKg4Dxi+y{d`6ZX6S1$ zz}C7z&07E2!cW%(k4)XWXFF!7AJ)qnb$HIkmD7_hZ~kOl5(@K}p)<^|)Af(Wl)ila zT0zU4mrAmIGG-`FLTLPH`Rb<1hjMC^HES-O?C@ZQ9tW9nJa~Qm*|wRxx11`wl6=64 z8CqyT$Z_BD_S!w_(Vq|9|3-7|7fqO<&s!17FH70bnVtXs>e-FQTU$1szzjVHGCS?q z$w#KzTAJIV>kj2KeU2Hr%Z5;0{*}nsj_X^C=lOqw@QeAMOG`}59__fUy$$oDEX6WQ}gtjg@ zu*_Y$MG_im0rC1ZMU}!8UH$Ti%1ff+ zjBx%2m{X!snNua%OL&Ed5)mXZNM*u_L}CM-q@<3-B#nra28bHsm0S`LD5p>uKt+8Z zuP7RI4y8fiL=2KCCqy_kG^{OYNW>CeDdq(_tnG*i}g_6sC^>73|Ch?}>|KvXYPx6DmjZRjTyQ0WbzHo8TyxNkoMdcNh zeMZ~(a72>f-Zg+Rx@#O=s6&)QN0?=cvtGGPoSW=*lH}57+x0lc*9{O~jNEd`T;kJI z;+Hfb0D)JDkCHf10;F?wGf{!zDlY~JuM$qt15KOYtRg4Cnh^Q`75%Y3=So&2P4;&m z%VwMk;dG4cP|b8}RLKME5QBXdFl(}4w97)O8c_;dE?E);EmE&IB{}E{YCsAX6}C z@iH7yl4FCH;&+*8776|u$m<}*kX_4fpJ7P@Wmx*6g;n!Q+_lw(748!E{DJ*tT8nx+ ztTIGz;j}Iy^>AF5qeuwyz(>I)Kr7I3kYY|!1%C4Op25QdmrJ4{-X9`i*g(%M@FJ)C z#@RPzB7zp=MWdV6^NcKQHsOCvn>%Ck5YSIZgnm9?QGg%BqEb)Gk^$55uHKFzT6PHB zF5f{-5A_M$gM*p(BOZsp8-xd03(^X55abNVA0V0dHg|#z;WXX8ZceALw9s$rhvO=Y z`|I)zovFWWx}h^0y3{Tm4_AMap-k$YSjPzp?X{uB(2Y=Kt+y(A$i?uMR{-Zw_Z$`n^}T2n^_{+(Q9e5a S$p7u6-$SBDsyUCYx&hIZK*UD;@qf4ty4R=3~s!eT4{ZUUT`LXP-RQ)B5ex zvEXylCu5GTEkbDP?MGg%m-_4_`o7yfJQ!GmIr>g9Lfe*ox+1V}*`rtfSu6Dy^)+LT zY7-IK8T-B|9^D+Yc$$u;Ti)M;Ir>-$LT~T>amkAQYfERI*`yw8w17now8DkZ-YVPF z*%jN%U#sofTl?FE7cfU#f!_ToP(1hH^OvqZIPvfK=+`>T(P^a!on2G0aoI(5rhi&c zTfWVD0dsUEkQF(&fAM5&nFZqh&XBsrB2pUsDL{y2) zh{WU=Rfr%H#y9#%LkCGSoye3%i53^tN|KbQpiDonwC^+{Cl zA&9bpj%AJ|^Q+|ZMFm7;O8S;(nw`O@1HB1U581Wq#u;WCDZ@$@FKS)T5NKao+Z<>J zEEw5dmJMjI!CK<Bw7I!x9)s|c@(I5%L{U^6-@To36U{0- zyii$2Gm|Df4r2gF1ljW^RA(K)x2DnnEgHR`25h^c}R!C5=58H&e1Eeh#*0C N`7eWZv|>^c0017O$PEAh diff --git a/packages/node-resolve/test/test.js b/packages/node-resolve/test/test.js index 0979eb76a..45201bace 100755 --- a/packages/node-resolve/test/test.js +++ b/packages/node-resolve/test/test.js @@ -322,6 +322,39 @@ test('respects the package.json sideEffects property for files in root package b t.snapshot(code); }); +test('respects the package.json sideEffects property for files in the root package and does not do deep side effects by default', async (t) => { + const bundle = await rollup({ + input: 'deep-side-effects/index.js', + onwarn: failOnWarn(t), + plugins: [ + nodeResolve({ + rootDir: 'deep-side-effects' + }) + ] + }); + const code = await getCode(bundle); + t.true(code.includes('shallow side effect')); + t.false(code.includes('deep side effects')); + t.snapshot(code); +}); + +test('respects the package.json sideEffects property for files in the root package and supports deep side effects', async (t) => { + const bundle = await rollup({ + input: 'deep-side-effects/index.js', + onwarn: failOnWarn(t), + plugins: [ + nodeResolve({ + rootDir: 'deep-side-effects', + deepSideEffects: true + }) + ] + }); + const code = await getCode(bundle); + t.true(code.includes('shallow side effect')); + t.true(code.includes('deep side effect')); + t.snapshot(code); +}); + test('ignores the package.json sideEffects property for files in root package with "ignoreSideEffectsForRoot" option', async (t) => { const bundle = await rollup({ input: 'root-package-side-effect/index.js', From 8a4674e36e8d55f87322478bce720e867a1dc481 Mon Sep 17 00:00:00 2001 From: the_mcmurder <3059715+TheMcMurder@users.noreply.github.com> Date: Tue, 5 Apr 2022 16:27:35 -0600 Subject: [PATCH 2/5] chore(node-resolve): Fixing linting errors --- packages/node-resolve/README.md | 2 +- packages/node-resolve/src/index.js | 10 ++++++++-- .../node-resolve/src/resolveImportSpecifiers.js | 16 ++++++++-------- packages/node-resolve/src/util.js | 6 +++--- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/packages/node-resolve/README.md b/packages/node-resolve/README.md index e5e73552b..e6b9d7f0d 100755 --- a/packages/node-resolve/README.md +++ b/packages/node-resolve/README.md @@ -168,7 +168,7 @@ If you use the `sideEffects` property in the package.json, by default this is re Type: `Boolean`
Default: `false` -A boolean which instructs how to handle deep side effects. This is useful for packages that have nested files in their dist directory and also utilize `sideEffects` configuration in their package.json. +A boolean which instructs how to handle deep side effects. This is useful for packages that have nested files in their dist directory and also utilize `sideEffects` configuration in their package.json. ## Preserving symlinks diff --git a/packages/node-resolve/src/index.js b/packages/node-resolve/src/index.js index 862a07875..d4a4ca6e2 100644 --- a/packages/node-resolve/src/index.js +++ b/packages/node-resolve/src/index.js @@ -38,7 +38,7 @@ const defaults = { resolveOnly: [], moduleDirectories: ['node_modules'], ignoreSideEffectsForRoot: false, - deepSideEffects: false, + deepSideEffects: false }; export const DEFAULTS = deepFreeze(deepMerge({}, defaults)); @@ -46,7 +46,13 @@ export function nodeResolve(opts = {}) { const { warnings } = handleDeprecatedOptions(opts); const options = { ...defaults, ...opts }; - const { extensions, jail, moduleDirectories, ignoreSideEffectsForRoot, deepSideEffects } = options; + const { + extensions, + jail, + moduleDirectories, + ignoreSideEffectsForRoot, + deepSideEffects + } = options; const conditionsEsm = [...baseConditionsEsm, ...(options.exportConditions || [])]; const conditionsCjs = [...baseConditionsCjs, ...(options.exportConditions || [])]; const packageInfoCache = new Map(); diff --git a/packages/node-resolve/src/resolveImportSpecifiers.js b/packages/node-resolve/src/resolveImportSpecifiers.js index bcf718edf..3f72d3b90 100644 --- a/packages/node-resolve/src/resolveImportSpecifiers.js +++ b/packages/node-resolve/src/resolveImportSpecifiers.js @@ -115,7 +115,7 @@ async function resolveWithExportMap({ moduleDirectories, rootDir, ignoreSideEffectsForRoot, - deepSideEffects, + deepSideEffects }) { if (importSpecifier.startsWith('#')) { // this is a package internal import, resolve using package imports field @@ -133,7 +133,7 @@ async function resolveWithExportMap({ preserveSymlinks, useBrowserOverrides, baseDir, - moduleDirectories, + moduleDirectories }); } }); @@ -168,7 +168,7 @@ async function resolveWithExportMap({ useBrowserOverrides, rootDir, ignoreSideEffectsForRoot, - deepSideEffects, + deepSideEffects }); ({ packageInfo, hasModuleSideEffects, hasPackageEntry, packageBrowserField } = info); @@ -236,7 +236,7 @@ async function resolveWithClassic({ moduleDirectories, rootDir, ignoreSideEffectsForRoot, - deepSideEffects, + deepSideEffects }) { for (let i = 0; i < importSpecifierList.length; i++) { // eslint-disable-next-line no-await-in-loop @@ -254,7 +254,7 @@ async function resolveWithClassic({ moduleDirectories, rootDir, ignoreSideEffectsForRoot, - deepSideEffects, + deepSideEffects }); if (result) { @@ -283,7 +283,7 @@ export default async function resolveImportSpecifiers({ moduleDirectories, rootDir, ignoreSideEffectsForRoot, - deepSideEffects, + deepSideEffects }) { try { const exportMapRes = await resolveWithExportMap({ @@ -299,7 +299,7 @@ export default async function resolveImportSpecifiers({ moduleDirectories, rootDir, ignoreSideEffectsForRoot, - deepSideEffects, + deepSideEffects }); if (exportMapRes) return exportMapRes; } catch (error) { @@ -325,6 +325,6 @@ export default async function resolveImportSpecifiers({ moduleDirectories, rootDir, ignoreSideEffectsForRoot, - deepSideEffects, + deepSideEffects }); } diff --git a/packages/node-resolve/src/util.js b/packages/node-resolve/src/util.js index 07373e6e4..783ef6813 100644 --- a/packages/node-resolve/src/util.js +++ b/packages/node-resolve/src/util.js @@ -49,7 +49,7 @@ export function getPackageInfo(options) { useBrowserOverrides, rootDir, ignoreSideEffectsForRoot, - deepSideEffects, + deepSideEffects } = options; let { pkgPath } = options; @@ -145,9 +145,9 @@ export function getPackageInfo(options) { if (typeof packageSideEffects === 'boolean') { internalPackageInfo.hasModuleSideEffects = () => packageSideEffects; } else if (Array.isArray(packageSideEffects)) { - let _packageSideEffects = packageSideEffects + let _packageSideEffects = packageSideEffects; if (deepSideEffects) { - _packageSideEffects = _packageSideEffects.map((sideEffect) => `**/${sideEffect}`) + _packageSideEffects = _packageSideEffects.map((sideEffect) => `**/${sideEffect}`); } internalPackageInfo.hasModuleSideEffects = createFilter(_packageSideEffects, null, { resolve: pkgRoot From e7738eb4f20665fb9d8203b06e0a8c97b52e8c42 Mon Sep 17 00:00:00 2001 From: the_mcmurder <3059715+TheMcMurder@users.noreply.github.com> Date: Thu, 14 Apr 2022 08:42:55 -0600 Subject: [PATCH 3/5] feat: deep side effects - prevent changes when side effects start with "./" || "/" --- packages/node-resolve/src/util.js | 12 +++++++++--- .../deep/side-effect.js | 1 + .../index.js | 4 ++++ .../package.json | 4 ++++ .../shallow-side-effect.js | 1 + .../node-resolve/test/snapshots/test.js.md | 11 +++++++++++ .../node-resolve/test/snapshots/test.js.snap | Bin 1568 -> 1595 bytes packages/node-resolve/test/test.js | 17 +++++++++++++++++ 8 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 packages/node-resolve/test/fixtures/deep-side-effects-with-specific-side-effects/deep/side-effect.js create mode 100644 packages/node-resolve/test/fixtures/deep-side-effects-with-specific-side-effects/index.js create mode 100644 packages/node-resolve/test/fixtures/deep-side-effects-with-specific-side-effects/package.json create mode 100644 packages/node-resolve/test/fixtures/deep-side-effects-with-specific-side-effects/shallow-side-effect.js diff --git a/packages/node-resolve/src/util.js b/packages/node-resolve/src/util.js index 783ef6813..05e29b88b 100644 --- a/packages/node-resolve/src/util.js +++ b/packages/node-resolve/src/util.js @@ -145,11 +145,17 @@ export function getPackageInfo(options) { if (typeof packageSideEffects === 'boolean') { internalPackageInfo.hasModuleSideEffects = () => packageSideEffects; } else if (Array.isArray(packageSideEffects)) { - let _packageSideEffects = packageSideEffects; + let finalPackageSideEffects = packageSideEffects; if (deepSideEffects) { - _packageSideEffects = _packageSideEffects.map((sideEffect) => `**/${sideEffect}`); + finalPackageSideEffects = packageSideEffects.map((sideEffect) => { + // if the sideEffect starts with "./" or starts with "/" do nothing + if (sideEffect.indexOf('./') === 0 || sideEffect.indexOf('/') === 0) { + return sideEffect; + } + return `**/${sideEffect}`; + }); } - internalPackageInfo.hasModuleSideEffects = createFilter(_packageSideEffects, null, { + internalPackageInfo.hasModuleSideEffects = createFilter(finalPackageSideEffects, null, { resolve: pkgRoot }); } diff --git a/packages/node-resolve/test/fixtures/deep-side-effects-with-specific-side-effects/deep/side-effect.js b/packages/node-resolve/test/fixtures/deep-side-effects-with-specific-side-effects/deep/side-effect.js new file mode 100644 index 000000000..a9932c054 --- /dev/null +++ b/packages/node-resolve/test/fixtures/deep-side-effects-with-specific-side-effects/deep/side-effect.js @@ -0,0 +1 @@ +console.log('deep side effect') \ No newline at end of file diff --git a/packages/node-resolve/test/fixtures/deep-side-effects-with-specific-side-effects/index.js b/packages/node-resolve/test/fixtures/deep-side-effects-with-specific-side-effects/index.js new file mode 100644 index 000000000..2b08cba9b --- /dev/null +++ b/packages/node-resolve/test/fixtures/deep-side-effects-with-specific-side-effects/index.js @@ -0,0 +1,4 @@ +import './deep/side-effect.js' +import './shallow-side-effect.js' + +console.log('main') \ No newline at end of file diff --git a/packages/node-resolve/test/fixtures/deep-side-effects-with-specific-side-effects/package.json b/packages/node-resolve/test/fixtures/deep-side-effects-with-specific-side-effects/package.json new file mode 100644 index 000000000..1254bc8ae --- /dev/null +++ b/packages/node-resolve/test/fixtures/deep-side-effects-with-specific-side-effects/package.json @@ -0,0 +1,4 @@ +{ + "main": "./index.js", + "sideEffects": ["./*.js"] +} diff --git a/packages/node-resolve/test/fixtures/deep-side-effects-with-specific-side-effects/shallow-side-effect.js b/packages/node-resolve/test/fixtures/deep-side-effects-with-specific-side-effects/shallow-side-effect.js new file mode 100644 index 000000000..dd89c4709 --- /dev/null +++ b/packages/node-resolve/test/fixtures/deep-side-effects-with-specific-side-effects/shallow-side-effect.js @@ -0,0 +1 @@ +console.log('shallow side effect') \ No newline at end of file diff --git a/packages/node-resolve/test/snapshots/test.js.md b/packages/node-resolve/test/snapshots/test.js.md index 77aae2a80..f62176a25 100644 --- a/packages/node-resolve/test/snapshots/test.js.md +++ b/packages/node-resolve/test/snapshots/test.js.md @@ -200,3 +200,14 @@ Generated by [AVA](https://avajs.dev). ␊ console.log('main');␊ ` + +## does not prefix the sideEffects property if the property starts with "./" even if deep side effects is set + +> Snapshot 1 + + `'use strict';␊ + ␊ + console.log('shallow side effect');␊ + ␊ + console.log('main');␊ + ` diff --git a/packages/node-resolve/test/snapshots/test.js.snap b/packages/node-resolve/test/snapshots/test.js.snap index b5e0acb408a3e76151250697b72f215f2c6f9177..54057f058a8c254a3231b0e180021a27cd1ad230 100644 GIT binary patch literal 1595 zcmV-B2E_S6RzVijMHHUd?Y2u>`hZfw@R$g)g5BNNLXaXR1zKF7TkN(JBDKA{JKeqQ-Fuh&=yn4{ z(4Z6$mHJ541W{r`Kp{RLRU|+n4;4{@)er=&LKJ@}5Wxh4o;%&!^dV&-t@ejEIk_|6 znb$ezJ2S^`Lg)@;&ODfZ?Z;EqcGLtyLN8I4E4c!nZpmvTDM|q!o>|Ak4Zow9y4^B8KLZ}AC8*$ z;@KFS1PMlpFiH_!3;eDGU-U*>e|yS(|2qvrmD{xFhie7LMX2|xv@Pf@4Zzs>yK<{UUv*L^bE+% z)WgT`pKNVzYK=U#KfB>m%+T#tglh6Gg-5rwq*vEjmQ)`8`g_dK%4Ew%wL)l5X)DDHb)_S;HSkqQ(BJ4W z*-8#Z%ih_J89E{Zp*OdEyJ%_q^@^!y)+&dJO%OpH{R3o2Zo-%u<2Pr&Tu`^8;Mem{ zV}?%7MCk4B+^MsspSyT%%FsXOpq~pdLob7zT|WM)#TU?-_OTvy$tLr8%+S1H2$@k5 z;}afUY7X>eROK$y8zT+KL*5tfZa9?1}Lh``&8SE ziNCiNVl1UW&FB7F}EalELbof})RDF^a8IAaBQK(5^pUWL61qSY4|^} zhyN3N;BTXoS?(?^^pq@ER5+)~RlKmIw5-QyD<29=GTgfcFiLlgy#uw2l4uXHY;n{o zH;HqDtwxfZ`fQsX$Jn}l;*F9UE}28Tno4|#{y~Wv~=~EmbdkG4A8Oz;CA^IYI>+o;2!MHydUxq{M{ft$QqC>Ap1a0 zf&31Vfp2mrSP@Ru?d#-pGD{2nroJzx!nn6C&(N8A>!unyv!P4r(D88fHyFx<&WSag zpwMm`S`6I~Ro1$z5__p~4OLR-icH0J`=6*rUy8T(>~EI-SGMRyhp6by!E8&fXYdAr z0X>!LsV5o|c(JYzlPW0S5yA-(E~*fnnJPT29W@)N?!8WIUN=Th=!?<+23g~J%wo4{ t|2}59vpXyB*!INca6;j1BQ>%VBARGa<#s{xaROBCe*+W)xGt9z0077o02BZK literal 1568 zcmV+*2H*KXRzVZe+&@e4-ER*Z}R1y^ZU;0 z`@Y|I&R1$eCEG8N*1Xd%SkKg4Dxi+y{d`6ZX6S1$ zz}C7z&07E2!cW%(k4)XWXFF!7AJ)qnb$HIkmD7_hZ~kOl5(@K}p)<^|)Af(Wl)ila zT0zU4mrAmIGG-`FLTLPH`Rb<1hjMC^HES-O?C@ZQ9tW9nJa~Qm*|wRxx11`wl6=64 z8CqyT$Z_BD_S!w_(Vq|9|3-7|7fqO<&s!17FH70bnVtXs>e-FQTU$1szzjVHGCS?q z$w#KzTAJIV>kj2KeU2Hr%Z5;0{*}nsj_X^C=lOqw@QeAMOG`}59__fUy$$oDEX6WQ}gtjg@ zu*_Y$MG_im0rC1ZMU}!8UH$Ti%1ff+ zjBx%2m{X!snNua%OL&Ed5)mXZNM*u_L}CM-q@<3-B#nra28bHsm0S`LD5p>uKt+8Z zuP7RI4y8fiL=2KCCqy_kG^{OYNW>CeDdq(_tnG*i}g_6sC^>73|Ch?}>|KvXYPx6DmjZRjTyQ0WbzHo8TyxNkoMdcNh zeMZ~(a72>f-Zg+Rx@#O=s6&)QN0?=cvtGGPoSW=*lH}57+x0lc*9{O~jNEd`T;kJI z;+Hfb0D)JDkCHf10;F?wGf{!zDlY~JuM$qt15KOYtRg4Cnh^Q`75%Y3=So&2P4;&m z%VwMk;dG4cP|b8}RLKME5QBXdFl(}4w97)O8c_;dE?E);EmE&IB{}E{YCsAX6}C z@iH7yl4FCH;&+*8776|u$m<}*kX_4fpJ7P@Wmx*6g;n!Q+_lw(748!E{DJ*tT8nx+ ztTIGz;j}Iy^>AF5qeuwyz(>I)Kr7I3kYY|!1%C4Op25QdmrJ4{-X9`i*g(%M@FJ)C z#@RPzB7zp=MWdV6^NcKQHsOCvn>%Ck5YSIZgnm9?QGg%BqEb)Gk^$55uHKFzT6PHB zF5f{-5A_M$gM*p(BOZsp8-xd03(^X55abNVA0V0dHg|#z;WXX8ZceALw9s$rhvO=Y z`|I)zovFWWx}h^0y3{Tm4_AMap-k$YSjPzp?X{uB(2Y=Kt+y(A$i?uMR{-Zw_Z$`n^}T2n^_{+(Q9e5a S$ { + const bundle = await rollup({ + input: 'deep-side-effects-with-specific-side-effects/index.js', + onwarn: failOnWarn(t), + plugins: [ + nodeResolve({ + rootDir: 'deep-side-effects-with-specific-side-effects', + deepSideEffects: true + }) + ] + }); + const code = await getCode(bundle); + t.true(code.includes('shallow side effect')); + t.false(code.includes('deep side effects')); + t.snapshot(code); +}); + test('ignores the package.json sideEffects property for files in root package with "ignoreSideEffectsForRoot" option', async (t) => { const bundle = await rollup({ input: 'root-package-side-effect/index.js', From 2864b29bac56f8823480983219226d02833ad834 Mon Sep 17 00:00:00 2001 From: the_mcmurder <3059715+TheMcMurder@users.noreply.github.com> Date: Thu, 14 Apr 2022 08:56:57 -0600 Subject: [PATCH 4/5] feat: support deep side effects by default - matching webpack implementation --- packages/node-resolve/README.md | 7 ----- packages/node-resolve/src/index.js | 14 ++------- .../src/resolveImportSpecifiers.js | 27 ++++++------------ packages/node-resolve/src/util.js | 22 +++++++------- .../node-resolve/test/snapshots/test.js.md | 13 +-------- .../node-resolve/test/snapshots/test.js.snap | Bin 1595 -> 1618 bytes packages/node-resolve/test/test.js | 24 ++-------------- 7 files changed, 27 insertions(+), 80 deletions(-) diff --git a/packages/node-resolve/README.md b/packages/node-resolve/README.md index e6b9d7f0d..905d15046 100755 --- a/packages/node-resolve/README.md +++ b/packages/node-resolve/README.md @@ -163,13 +163,6 @@ rootDir: path.join(process.cwd(), '..') If you use the `sideEffects` property in the package.json, by default this is respected for files in the root package. Set to `true` to ignore the `sideEffects` configuration for the root package. -### `deepSideEffects` - -Type: `Boolean`
-Default: `false` - -A boolean which instructs how to handle deep side effects. This is useful for packages that have nested files in their dist directory and also utilize `sideEffects` configuration in their package.json. - ## Preserving symlinks This plugin honours the rollup [`preserveSymlinks`](https://rollupjs.org/guide/en/#preservesymlinks) option. diff --git a/packages/node-resolve/src/index.js b/packages/node-resolve/src/index.js index d4a4ca6e2..3eab84bfe 100644 --- a/packages/node-resolve/src/index.js +++ b/packages/node-resolve/src/index.js @@ -37,8 +37,7 @@ const defaults = { extensions: ['.mjs', '.js', '.json', '.node'], resolveOnly: [], moduleDirectories: ['node_modules'], - ignoreSideEffectsForRoot: false, - deepSideEffects: false + ignoreSideEffectsForRoot: false }; export const DEFAULTS = deepFreeze(deepMerge({}, defaults)); @@ -46,13 +45,7 @@ export function nodeResolve(opts = {}) { const { warnings } = handleDeprecatedOptions(opts); const options = { ...defaults, ...opts }; - const { - extensions, - jail, - moduleDirectories, - ignoreSideEffectsForRoot, - deepSideEffects - } = options; + const { extensions, jail, moduleDirectories, ignoreSideEffectsForRoot } = options; const conditionsEsm = [...baseConditionsEsm, ...(options.exportConditions || [])]; const conditionsCjs = [...baseConditionsCjs, ...(options.exportConditions || [])]; const packageInfoCache = new Map(); @@ -170,8 +163,7 @@ export function nodeResolve(opts = {}) { baseDir, moduleDirectories, rootDir, - ignoreSideEffectsForRoot, - deepSideEffects + ignoreSideEffectsForRoot }); const importeeIsBuiltin = builtins.has(importee); diff --git a/packages/node-resolve/src/resolveImportSpecifiers.js b/packages/node-resolve/src/resolveImportSpecifiers.js index 3f72d3b90..e0a5b0466 100644 --- a/packages/node-resolve/src/resolveImportSpecifiers.js +++ b/packages/node-resolve/src/resolveImportSpecifiers.js @@ -44,8 +44,7 @@ async function resolveIdClassic({ baseDir, moduleDirectories, rootDir, - ignoreSideEffectsForRoot, - deepSideEffects + ignoreSideEffectsForRoot }) { let hasModuleSideEffects = () => null; let hasPackageEntry = true; @@ -62,8 +61,7 @@ async function resolveIdClassic({ preserveSymlinks, useBrowserOverrides, rootDir, - ignoreSideEffectsForRoot, - deepSideEffects + ignoreSideEffectsForRoot }); ({ packageInfo, hasModuleSideEffects, hasPackageEntry, packageBrowserField } = info); @@ -114,8 +112,7 @@ async function resolveWithExportMap({ baseDir, moduleDirectories, rootDir, - ignoreSideEffectsForRoot, - deepSideEffects + ignoreSideEffectsForRoot }) { if (importSpecifier.startsWith('#')) { // this is a package internal import, resolve using package imports field @@ -167,8 +164,7 @@ async function resolveWithExportMap({ preserveSymlinks, useBrowserOverrides, rootDir, - ignoreSideEffectsForRoot, - deepSideEffects + ignoreSideEffectsForRoot }); ({ packageInfo, hasModuleSideEffects, hasPackageEntry, packageBrowserField } = info); @@ -235,8 +231,7 @@ async function resolveWithClassic({ baseDir, moduleDirectories, rootDir, - ignoreSideEffectsForRoot, - deepSideEffects + ignoreSideEffectsForRoot }) { for (let i = 0; i < importSpecifierList.length; i++) { // eslint-disable-next-line no-await-in-loop @@ -253,8 +248,7 @@ async function resolveWithClassic({ baseDir, moduleDirectories, rootDir, - ignoreSideEffectsForRoot, - deepSideEffects + ignoreSideEffectsForRoot }); if (result) { @@ -282,8 +276,7 @@ export default async function resolveImportSpecifiers({ baseDir, moduleDirectories, rootDir, - ignoreSideEffectsForRoot, - deepSideEffects + ignoreSideEffectsForRoot }) { try { const exportMapRes = await resolveWithExportMap({ @@ -298,8 +291,7 @@ export default async function resolveImportSpecifiers({ baseDir, moduleDirectories, rootDir, - ignoreSideEffectsForRoot, - deepSideEffects + ignoreSideEffectsForRoot }); if (exportMapRes) return exportMapRes; } catch (error) { @@ -324,7 +316,6 @@ export default async function resolveImportSpecifiers({ baseDir, moduleDirectories, rootDir, - ignoreSideEffectsForRoot, - deepSideEffects + ignoreSideEffectsForRoot }); } diff --git a/packages/node-resolve/src/util.js b/packages/node-resolve/src/util.js index 05e29b88b..4629ae8e8 100644 --- a/packages/node-resolve/src/util.js +++ b/packages/node-resolve/src/util.js @@ -48,8 +48,7 @@ export function getPackageInfo(options) { preserveSymlinks, useBrowserOverrides, rootDir, - ignoreSideEffectsForRoot, - deepSideEffects + ignoreSideEffectsForRoot } = options; let { pkgPath } = options; @@ -146,15 +145,16 @@ export function getPackageInfo(options) { internalPackageInfo.hasModuleSideEffects = () => packageSideEffects; } else if (Array.isArray(packageSideEffects)) { let finalPackageSideEffects = packageSideEffects; - if (deepSideEffects) { - finalPackageSideEffects = packageSideEffects.map((sideEffect) => { - // if the sideEffect starts with "./" or starts with "/" do nothing - if (sideEffect.indexOf('./') === 0 || sideEffect.indexOf('/') === 0) { - return sideEffect; - } - return `**/${sideEffect}`; - }); - } + finalPackageSideEffects = packageSideEffects.map((sideEffect) => { + /* + * The array accepts simple glob patterns to the relevant files... Patterns like .css, which do not include a /, will be treated like **\/.css. + * https://webpack.js.org/guides/tree-shaking/ + */ + if (sideEffect.includes('/')) { + return sideEffect; + } + return `**/${sideEffect}`; + }); internalPackageInfo.hasModuleSideEffects = createFilter(finalPackageSideEffects, null, { resolve: pkgRoot }); diff --git a/packages/node-resolve/test/snapshots/test.js.md b/packages/node-resolve/test/snapshots/test.js.md index f62176a25..de7267431 100644 --- a/packages/node-resolve/test/snapshots/test.js.md +++ b/packages/node-resolve/test/snapshots/test.js.md @@ -177,17 +177,6 @@ Generated by [AVA](https://avajs.dev). message: 'node-resolve: `customResolveOptions.packageIterator` is no longer an option. If you need this, please open an issue.', } -## respects the package.json sideEffects property for files in the root package and does not do deep side effects by default - -> Snapshot 1 - - `'use strict';␊ - ␊ - console.log('shallow side effect');␊ - ␊ - console.log('main');␊ - ` - ## respects the package.json sideEffects property for files in the root package and supports deep side effects > Snapshot 1 @@ -201,7 +190,7 @@ Generated by [AVA](https://avajs.dev). console.log('main');␊ ` -## does not prefix the sideEffects property if the property starts with "./" even if deep side effects is set +## does not prefix the sideEffects property if the side effect contains a "/" > Snapshot 1 diff --git a/packages/node-resolve/test/snapshots/test.js.snap b/packages/node-resolve/test/snapshots/test.js.snap index 54057f058a8c254a3231b0e180021a27cd1ad230..83f4e677a760ab0f96a3c69163da2314b484aff0 100644 GIT binary patch literal 1618 zcmV-Y2Cex)RzVj*X zZ2?iKk5o+%B{l>UqC8SX0wnTK5hYj+LBJ|R@ec(QOfcxT)7z#GDFjUUfHy3%`a%8u!Ckie{ z@3mpZW=A1py=!S(#dh`3cl+;tqq*YKI?UK-V-U(Lh;8af$=tbOR_&2ZEo+Wp#-0Jp zia&h(fhp#e=CU4vQwnHiU2z(6sEzJx@MzsJZm>C%nZ^ zO-#d#eKQ`RwWAKaoF}wLC1`Jt*wyA-h8cT&I6~`w`lU%ISDrt8VDF=ojy5)9#!gB= zXnk4MvHZ+7H2#NHdm!b#d6=;cfDMcGE^+2Bdg$t(%Y}}(_9Dz!Mf#L%7cgVj03uKfQyNagrs?pU1sI7<$v7%kmL+*4jy9SY8?B_IB!q_} z3<^oqJE!0XgrkyijGrnB=cUHGDoP$|m8l{LwKR)Vx-~_W{6(Gbg#ne9M8y{1+%+)9 zp;DPsCApID3K1nDNTQd@gcFIxI@*Y%iquOQ5h?W$)yFI8Bp^^uq0oVfdPiPSG-?}6 zhy39>3>H5T!l1rUMxy6O^7v|b@goNTD9V?oN3JO1b9~ohdcvX`)-#H;(A`0%`3bj$pH@37LMBzS6o*8zD@T}y9WVUhh+Si*w&rE?w5in8n?r^7k7|9F{Jqn-gP@zI+E zty4(d0@oQR;)6PHQ3wgh3Tzydn2nS|oGN>c;NgHvCqW7nAPL`MPcS1Kulv`f=~z}4`c8d+$b@lUTc%+% z^|ehiY!QYnuG7ZD)?a5BBfADxae_j7d@#ze4O3;UrzyIRDZ?}&rE)p(b} zy?YL{O8+fi^h<}R=*^+*OP^!#CW!$%m1}8nyijMHHUd?Y2u>`hZfw@R$g)g5BNNLXaXR1zKF7TkN(JBDKA{JKeqQ-Fuh&=yn4{ z(4Z6$mHJ541W{r`Kp{RLRU|+n4;4{@)er=&LKJ@}5Wxh4o;%&!^dV&-t@ejEIk_|6 znb$ezJ2S^`Lg)@;&ODfZ?Z;EqcGLtyLN8I4E4c!nZpmvTDM|q!o>|Ak4Zow9y4^B8KLZ}AC8*$ z;@KFS1PMlpFiH_!3;eDGU-U*>e|yS(|2qvrmD{xFhie7LMX2|xv@Pf@4Zzs>yK<{UUv*L^bE+% z)WgT`pKNVzYK=U#KfB>m%+T#tglh6Gg-5rwq*vEjmQ)`8`g_dK%4Ew%wL)l5X)DDHb)_S;HSkqQ(BJ4W z*-8#Z%ih_J89E{Zp*OdEyJ%_q^@^!y)+&dJO%OpH{R3o2Zo-%u<2Pr&Tu`^8;Mem{ zV}?%7MCk4B+^MsspSyT%%FsXOpq~pdLob7zT|WM)#TU?-_OTvy$tLr8%+S1H2$@k5 z;}afUY7X>eROK$y8zT+KL*5tfZa9?1}Lh``&8SE ziNCiNVl1UW&FB7F}EalELbof})RDF^a8IAaBQK(5^pUWL61qSY4|^} zhyN3N;BTXoS?(?^^pq@ER5+)~RlKmIw5-QyD<29=GTgfcFiLlgy#uw2l4uXHY;n{o zH;HqDtwxfZ`fQsX$Jn}l;*F9UE}28Tno4|#{y~Wv~=~EmbdkG4A8Oz;CA^IYI>+o;2!MHydUxq{M{ft$QqC>Ap1a0 zf&31Vfp2mrSP@Ru?d#-pGD{2nroJzx!nn6C&(N8A>!unyv!P4r(D88fHyFx<&WSag zpwMm`S`6I~Ro1$z5__p~4OLR-icH0J`=6*rUy8T(>~EI-SGMRyhp6by!E8&fXYdAr z0X>!LsV5o|c(JYzlPW0S5yA-(E~*fnnJPT29W@)N?!8WIUN=Th=!?<+23g~J%wo4{ t|2}59vpXyB*!INca6;j1BQ>%VBARGa<#s{xaROBCe*+W)xGt9z0077o02BZK diff --git a/packages/node-resolve/test/test.js b/packages/node-resolve/test/test.js index 016771dd9..50cbcac82 100755 --- a/packages/node-resolve/test/test.js +++ b/packages/node-resolve/test/test.js @@ -322,30 +322,13 @@ test('respects the package.json sideEffects property for files in root package b t.snapshot(code); }); -test('respects the package.json sideEffects property for files in the root package and does not do deep side effects by default', async (t) => { - const bundle = await rollup({ - input: 'deep-side-effects/index.js', - onwarn: failOnWarn(t), - plugins: [ - nodeResolve({ - rootDir: 'deep-side-effects' - }) - ] - }); - const code = await getCode(bundle); - t.true(code.includes('shallow side effect')); - t.false(code.includes('deep side effects')); - t.snapshot(code); -}); - test('respects the package.json sideEffects property for files in the root package and supports deep side effects', async (t) => { const bundle = await rollup({ input: 'deep-side-effects/index.js', onwarn: failOnWarn(t), plugins: [ nodeResolve({ - rootDir: 'deep-side-effects', - deepSideEffects: true + rootDir: 'deep-side-effects' }) ] }); @@ -355,14 +338,13 @@ test('respects the package.json sideEffects property for files in the root packa t.snapshot(code); }); -test('does not prefix the sideEffects property if the property starts with "./" even if deep side effects is set', async (t) => { +test('does not prefix the sideEffects property if the side effect contains a "/"', async (t) => { const bundle = await rollup({ input: 'deep-side-effects-with-specific-side-effects/index.js', onwarn: failOnWarn(t), plugins: [ nodeResolve({ - rootDir: 'deep-side-effects-with-specific-side-effects', - deepSideEffects: true + rootDir: 'deep-side-effects-with-specific-side-effects' }) ] }); From 1de53732098522a74e110f9d7889965f26a46c18 Mon Sep 17 00:00:00 2001 From: the_mcmurder <3059715+TheMcMurder@users.noreply.github.com> Date: Thu, 14 Apr 2022 11:31:23 -0600 Subject: [PATCH 5/5] chore(cleanup): pr feedback --- packages/node-resolve/src/util.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/node-resolve/src/util.js b/packages/node-resolve/src/util.js index 4629ae8e8..c61deb27d 100644 --- a/packages/node-resolve/src/util.js +++ b/packages/node-resolve/src/util.js @@ -144,8 +144,7 @@ export function getPackageInfo(options) { if (typeof packageSideEffects === 'boolean') { internalPackageInfo.hasModuleSideEffects = () => packageSideEffects; } else if (Array.isArray(packageSideEffects)) { - let finalPackageSideEffects = packageSideEffects; - finalPackageSideEffects = packageSideEffects.map((sideEffect) => { + const finalPackageSideEffects = packageSideEffects.map((sideEffect) => { /* * The array accepts simple glob patterns to the relevant files... Patterns like .css, which do not include a /, will be treated like **\/.css. * https://webpack.js.org/guides/tree-shaking/