Skip to content

Commit

Permalink
fix(node-resolve): update side effects logic to be deep when glob doe…
Browse files Browse the repository at this point in the history
…sn’t contain `/` (#1148)

* feat(node-resolve): deep side effects option

* chore(node-resolve): Fixing linting errors

* feat: deep side effects - prevent changes when side effects start with "./" || "/"

* feat: support deep side effects by default - matching webpack implementation

* chore(cleanup): pr feedback
  • Loading branch information
TheMcMurder authored Apr 15, 2022
1 parent 2115a00 commit 18afe15
Show file tree
Hide file tree
Showing 12 changed files with 87 additions and 1 deletion.
12 changes: 11 additions & 1 deletion packages/node-resolve/src/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,17 @@ export function getPackageInfo(options) {
if (typeof packageSideEffects === 'boolean') {
internalPackageInfo.hasModuleSideEffects = () => packageSideEffects;
} else if (Array.isArray(packageSideEffects)) {
internalPackageInfo.hasModuleSideEffects = createFilter(packageSideEffects, null, {
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/
*/
if (sideEffect.includes('/')) {
return sideEffect;
}
return `**/${sideEffect}`;
});
internalPackageInfo.hasModuleSideEffects = createFilter(finalPackageSideEffects, null, {
resolve: pkgRoot
});
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log('deep side effect')
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import './deep/side-effect.js'
import './shallow-side-effect.js'

console.log('main')
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"main": "./index.js",
"sideEffects": ["./*.js"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log('shallow side effect')
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log('deep side effect')
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import './deep/side-effect.js'
import './shallow-side-effect.js'

console.log('main')
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"main": "./index.js",
"sideEffects": ["*.js"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log('shallow side effect')
24 changes: 24 additions & 0 deletions packages/node-resolve/test/snapshots/test.js.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 supports deep side effects

> Snapshot 1
`'use strict';␊
console.log('deep side effect');␊
console.log('shallow side effect');␊
console.log('main');␊
`

## does not prefix the sideEffects property if the side effect contains a "/"

> Snapshot 1
`'use strict';␊
console.log('shallow side effect');␊
console.log('main');␊
`
Binary file modified packages/node-resolve/test/snapshots/test.js.snap
Binary file not shown.
32 changes: 32 additions & 0 deletions packages/node-resolve/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,38 @@ 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 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'
})
]
});
const code = await getCode(bundle);
t.true(code.includes('shallow side effect'));
t.true(code.includes('deep side effect'));
t.snapshot(code);
});

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'
})
]
});
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',
Expand Down

0 comments on commit 18afe15

Please sign in to comment.