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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 11 additions & 1 deletion packages/node-resolve/src/util.js
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
@@ -0,0 +1 @@
console.log('deep side effect')
@@ -0,0 +1,4 @@
import './deep/side-effect.js'
import './shallow-side-effect.js'

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

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