Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(node-resolve): update side effects logic to be deep when glob doesn’t contain / #1148

Merged
merged 5 commits into from
Apr 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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