From 7359d1f24ea0f53fdd3e3043daacb742603d74ba Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Sun, 14 Feb 2021 14:09:38 +0000 Subject: [PATCH] feat(node-resolve): add `ignoreSideEffectsForRoot` option (#694) * add test for root package side effect import with package sideEffect option * ignore package `sideEffects` property for root package * remove package name * add `ignoreSideEffectsForRoot` option * make test fixture a bit more accurate --- packages/node-resolve/README.md | 4 +++ packages/node-resolve/src/index.js | 11 ++++--- .../src/resolveImportSpecifiers.js | 16 ++++++--- packages/node-resolve/src/util.js | 27 ++++++++++----- .../root-package-side-effect/index.js | 1 + .../root-package-side-effect/package.json | 4 +++ .../root-package-side-effect/side-effect.js | 1 + .../node-resolve/test/snapshots/test.js.md | 17 ++++++++++ .../node-resolve/test/snapshots/test.js.snap | Bin 1348 -> 1437 bytes packages/node-resolve/test/test.js | 31 ++++++++++++++++++ 10 files changed, 96 insertions(+), 16 deletions(-) create mode 100644 packages/node-resolve/test/fixtures/root-package-side-effect/index.js create mode 100644 packages/node-resolve/test/fixtures/root-package-side-effect/package.json create mode 100644 packages/node-resolve/test/fixtures/root-package-side-effect/side-effect.js diff --git a/packages/node-resolve/README.md b/packages/node-resolve/README.md index ea195a06e..5993f3089 100755 --- a/packages/node-resolve/README.md +++ b/packages/node-resolve/README.md @@ -157,6 +157,10 @@ Specifies the root directory from which to resolve modules. Typically used when rootDir: path.join(process.cwd(), '..') ``` +## `ignoreSideEffectsForRoot` + +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. + ## 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 481fa8a60..a1861864b 100644 --- a/packages/node-resolve/src/index.js +++ b/packages/node-resolve/src/index.js @@ -34,7 +34,8 @@ const defaults = { // which deploy both ESM .mjs and CommonJS .js files as ESM. extensions: ['.mjs', '.js', '.json', '.node'], resolveOnly: [], - moduleDirectories: ['node_modules'] + moduleDirectories: ['node_modules'], + ignoreSideEffectsForRoot: false }; export const DEFAULTS = deepFreeze(deepMerge({}, defaults)); @@ -42,7 +43,7 @@ export function nodeResolve(opts = {}) { const { warnings } = handleDeprecatedOptions(opts); const options = { ...defaults, ...opts }; - const { extensions, jail, moduleDirectories } = options; + const { extensions, jail, moduleDirectories, ignoreSideEffectsForRoot } = options; const conditionsEsm = [...baseConditionsEsm, ...(options.exportConditions || [])]; const conditionsCjs = [...baseConditionsCjs, ...(options.exportConditions || [])]; const packageInfoCache = new Map(); @@ -51,7 +52,7 @@ export function nodeResolve(opts = {}) { const useBrowserOverrides = mainFields.indexOf('browser') !== -1; const isPreferBuiltinsSet = options.preferBuiltins === true || options.preferBuiltins === false; const preferBuiltins = isPreferBuiltinsSet ? options.preferBuiltins : true; - const rootDir = options.rootDir || process.cwd(); + const rootDir = resolve(options.rootDir || process.cwd()); let { dedupe } = options; let rollupOptions; @@ -200,7 +201,9 @@ export function nodeResolve(opts = {}) { preserveSymlinks, useBrowserOverrides, baseDir, - moduleDirectories + moduleDirectories, + rootDir, + ignoreSideEffectsForRoot }); const resolved = diff --git a/packages/node-resolve/src/resolveImportSpecifiers.js b/packages/node-resolve/src/resolveImportSpecifiers.js index 391e533f7..6c0904bf8 100644 --- a/packages/node-resolve/src/resolveImportSpecifiers.js +++ b/packages/node-resolve/src/resolveImportSpecifiers.js @@ -43,7 +43,9 @@ async function resolveId({ preserveSymlinks, useBrowserOverrides, baseDir, - moduleDirectories + moduleDirectories, + rootDir, + ignoreSideEffectsForRoot }) { let hasModuleSideEffects = () => null; let hasPackageEntry = true; @@ -58,7 +60,9 @@ async function resolveId({ pkgPath, mainFields, preserveSymlinks, - useBrowserOverrides + useBrowserOverrides, + rootDir, + ignoreSideEffectsForRoot }); ({ packageInfo, hasModuleSideEffects, hasPackageEntry, packageBrowserField } = info); @@ -180,7 +184,9 @@ export default async function resolveImportSpecifiers({ preserveSymlinks, useBrowserOverrides, baseDir, - moduleDirectories + moduleDirectories, + rootDir, + ignoreSideEffectsForRoot }) { let lastResolveError; @@ -197,7 +203,9 @@ export default async function resolveImportSpecifiers({ preserveSymlinks, useBrowserOverrides, baseDir, - moduleDirectories + moduleDirectories, + rootDir, + ignoreSideEffectsForRoot }); if (result instanceof ResolveError) { diff --git a/packages/node-resolve/src/util.js b/packages/node-resolve/src/util.js index b261b40f5..7b21d250a 100644 --- a/packages/node-resolve/src/util.js +++ b/packages/node-resolve/src/util.js @@ -40,7 +40,16 @@ export function getMainFields(options) { } export function getPackageInfo(options) { - const { cache, extensions, pkg, mainFields, preserveSymlinks, useBrowserOverrides } = options; + const { + cache, + extensions, + pkg, + mainFields, + preserveSymlinks, + useBrowserOverrides, + rootDir, + ignoreSideEffectsForRoot + } = options; let { pkgPath } = options; if (cache.has(pkgPath)) { @@ -130,13 +139,15 @@ export function getPackageInfo(options) { packageInfo.browserMappedMain = false; } - const packageSideEffects = pkg.sideEffects; - if (typeof packageSideEffects === 'boolean') { - internalPackageInfo.hasModuleSideEffects = () => packageSideEffects; - } else if (Array.isArray(packageSideEffects)) { - internalPackageInfo.hasModuleSideEffects = createFilter(packageSideEffects, null, { - resolve: pkgRoot - }); + if (!ignoreSideEffectsForRoot || rootDir !== pkgRoot) { + const packageSideEffects = pkg.sideEffects; + if (typeof packageSideEffects === 'boolean') { + internalPackageInfo.hasModuleSideEffects = () => packageSideEffects; + } else if (Array.isArray(packageSideEffects)) { + internalPackageInfo.hasModuleSideEffects = createFilter(packageSideEffects, null, { + resolve: pkgRoot + }); + } } cache.set(pkgPath, internalPackageInfo); diff --git a/packages/node-resolve/test/fixtures/root-package-side-effect/index.js b/packages/node-resolve/test/fixtures/root-package-side-effect/index.js new file mode 100644 index 000000000..3425e6f0f --- /dev/null +++ b/packages/node-resolve/test/fixtures/root-package-side-effect/index.js @@ -0,0 +1 @@ +import './side-effect.js'; diff --git a/packages/node-resolve/test/fixtures/root-package-side-effect/package.json b/packages/node-resolve/test/fixtures/root-package-side-effect/package.json new file mode 100644 index 000000000..dca881437 --- /dev/null +++ b/packages/node-resolve/test/fixtures/root-package-side-effect/package.json @@ -0,0 +1,4 @@ +{ + "main": "./index.js", + "sideEffects": false +} diff --git a/packages/node-resolve/test/fixtures/root-package-side-effect/side-effect.js b/packages/node-resolve/test/fixtures/root-package-side-effect/side-effect.js new file mode 100644 index 000000000..75bb07d82 --- /dev/null +++ b/packages/node-resolve/test/fixtures/root-package-side-effect/side-effect.js @@ -0,0 +1 @@ +console.log('side effect'); diff --git a/packages/node-resolve/test/snapshots/test.js.md b/packages/node-resolve/test/snapshots/test.js.md index 5a676b49b..eb686f717 100644 --- a/packages/node-resolve/test/snapshots/test.js.md +++ b/packages/node-resolve/test/snapshots/test.js.md @@ -47,6 +47,23 @@ Generated by [AVA](https://avajs.dev). }, ] +## ignores the package.json sideEffects property for files in root package with "ignoreSideEffectsForRoot" option + +> Snapshot 1 + + `'use strict';␊ + ␊ + console.log('side effect');␊ + ` + +## respects the package.json sideEffects property for files in root package by default + +> Snapshot 1 + + `'use strict';␊ + ␊ + ` + ## throws error for removed customResolveOptions.basedir option > Snapshot 1 diff --git a/packages/node-resolve/test/snapshots/test.js.snap b/packages/node-resolve/test/snapshots/test.js.snap index 385df660deab2b0f58623045f1ec5e5c4ecf13a8..5998414a57eb36bd4d268510715b8aadc16262c9 100644 GIT binary patch literal 1437 zcmV;O1!DR^RzV!rI17A`T%Lu;Gw(xNbZW=6ZYYX?wTZd&_;$HDCk{ z0-}jwJ~Cqh7$PB{s8I<%2#^RqgMZ+RB!Uhhk$()L_y-33?rCqc2S^t+)+S%hIlu3m zbHCsB{qE^KzhZ={&})0XTefQO>hf8qH*3dQY})~Z-&wGDpOkriS^uN1&?^T%iazl^ zX6Qbc@W%Jy^7#*(yLjckia!>jpBgbk4I4tc5?{9_g)I@Azx8Og{q4P&p$&F~d{sy0 zKfP|2dwiSp%rd~wypSZRe14=d6%!PlLp5Rv|)yR3pVYfdTDF> z%<4{U!-n%G1|pcDJH{bo_Z_Xj^5d!K?e5)sB3CEBhZ(xs1Cg(tzG=k;bb4@FL|?hh zejYRQEwI3?s}FYV*N>e&e8;POU0?KKhF&X2sIJAcWpHBMJL~7J`DADR(_dnS&aQxb z*E*)onZCXH<%XVp4L_fM7BjR1EME6Z`nG}H-e`|=W#{p4e!vWMc@f$=>Bvh>(tvY< z@%k}*t3MXkbRRu^NdS6Nw!a0S8`VXrxP zBDN#!^kG+uY8scI`LS&yk`;jlRH`Y`8d^`HyrJnzsx3EeP3xi}Yr!=5sk(+nvMEWF zd$cGKH6klSQse|x2`3YU%?y&|6z~k;qzsqUh@nxUCn-@CNixzTI(nb{x_^-(mWp8* z<1{LW>Wzst$4OQ(h)k&fltitDq$LWAU;>rRiK3<%^uH&P=J+01Q_@6~Vu;XQoywZo zEom;9Vz`JhFBt+|ps2J(5e$izHm6iEoK(sQO`=rVO6w`AI$Fu|NEgRrp{P<`SJZ4t zlPOkSQkv6~CCx+~!-)(U(`jk{!4!2an??B_aOp0VOKBg)`WO_}3^n24?f>TpIgt<7 zE>H|ffcNnBM@#E|o_5Rep}lY-q^VZ9sMYM`xU2{E(KpsHM!Wr6j) zUN6EPmWA;cKWHs-WuScwY%kb3vky0wU0{cBextJ1ST~$>W|H$=E1_j066)X~f!4s+ z5pHXYv@TiJxUj3aWoc_$`-s_YF_l(S$fE^J5hzUua%ezSO-O22Yt5|RMbQ$ zVTb|^Q8_d{!>Kat5U>KxQwwlfz=||5G}0;TJ;{~S-DJqTDN}48GJvuUQ{zm(d8g1I zP+s@X@pyR9zXHjVl*HYB4dNg)9tS!2@0sHnw$M9b;osG$Q-Rh1+X2=Dx?Fv8Z94u+ zn-i8U=~&bp?pof^7H$qN8sojK9d23IKOAx;81iM1I}At?6cnTI+CxFW$ALJ5qyusk z9lk<@kJTEINs4@uq~HOjDnWUinVaB9v85#=Am`9f*k@o1@;?QCB8{D-3XVP4=qIFVT4R|-kWz}Jla=h`D~+ovn-$8@{P~=M38&0 r<#ZGl#yLr&!yf3geB*T0815-7#=dOi6QFp3k=G{N3oEH-K*B!#wEq*v^@L>} z7)67C_`y)rkeCRD*br32L`nTa3W*T^B=`YqBmuMtiF`vMe!zfl=WZ{h1-V9z=O!;R z^LsOI-tWCPv%8}hAs>2Y|CzPx$FFzRp53M&Z*jZ@AUx+mdWY2XuC=?LaR=W#{CVVs zPq0AWg$eKf7%E@>_=U^Y9+~w=1Nx;A3)FNVv@iZ$YeL>0c2u<<%d~%Z01LF<2|LX` zy8Pu$wI!Ed`QqLZlvJ=l=awKe@App^ta|<2)wXf z%#G0YjaMFg@#*739p63|55M%ld@Rse9)xyweYQTds%zQR8=KVevavQS&>uh+p5j+_ zv_CYjTi?9-;>ocv7U-TC2swSn>aP8KI&z<9cz^i%{Ex9f2WEoj8y9Wux`fV-FAN*& zb~-O&fgS?!-#swWbI>?`e)RsghkCvl!~(rh4zXVHYwDh{VQ-}0wXXZb_dj8Q9;!fS z$K0cDG^t~*3iG|YJ{k#a!~#9z1rLy0(?Vh(TT>0%CtD;MA8ToGXtcP0-h$4nwBy;F?C9e1SOoQs+l5@sdjSf z{zZydErelI(y6TQ+Y@W z5TSzx)pV;{)Lhbqa8b0FYRYs4&OQ?d|FoJw+&q84|h^%s;Kz36$Q3ggkB za2hih&lEM8Ld#1{Nk*cmneb`c$WUVjE$$agLHDwil>Y&jzCyVa_f>36qr#dYClZ|g ze~yp~`Ebn&W~wsSGf2)2QWXgHGFC-;IZF~#%fYf=Wid$&^y&Ha9_*-wwnhZRib6f- zQbt5U^t@g#!ZSq_$}oPyUgSnMc-3ZVm?1Z5N;9dK$RH?L6Ov$C3`6G|*WlCXhQKTso*6`R21vPh|BT+!@d`9vQ@ zY3#taERE@AY{rw&hCmdMEg*YAj)0s7`3^WMx9gseJxIH$sRs&0v` zb4=CM+B&DLE6eH>D9k}yS(0DaE2%oYX+xK-n_+PCW>x7FRgJB3=Pe4A*zHd|tF#rJ G5dZ+b?u+37 diff --git a/packages/node-resolve/test/test.js b/packages/node-resolve/test/test.js index 3124b764f..7a279a467 100755 --- a/packages/node-resolve/test/test.js +++ b/packages/node-resolve/test/test.js @@ -246,6 +246,37 @@ test('resolves dynamic imports', async (t) => { t.is(result.default, 42); }); +test('respects the package.json sideEffects property for files in root package by default', async (t) => { + const bundle = await rollup({ + input: 'root-package-side-effect/index.js', + plugins: [ + nodeResolve({ + rootDir: 'root-package-side-effect' + }) + ] + }); + + const code = await getCode(bundle); + t.false(code.includes('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', + plugins: [ + nodeResolve({ + rootDir: 'root-package-side-effect', + ignoreSideEffectsForRoot: true + }) + ] + }); + + const code = await getCode(bundle); + t.true(code.includes('side effect')); + t.snapshot(code); +}); + test('handles package side-effects', async (t) => { const bundle = await rollup({ input: 'side-effects.js',