Skip to content

Commit

Permalink
feat(node-resolve): add ignoreSideEffectsForRoot option (#694)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
tjenkinson committed Feb 14, 2021
1 parent f29cee8 commit 7359d1f
Show file tree
Hide file tree
Showing 10 changed files with 96 additions and 16 deletions.
4 changes: 4 additions & 0 deletions packages/node-resolve/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 7 additions & 4 deletions packages/node-resolve/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,16 @@ 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));

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();
Expand All @@ -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;

Expand Down Expand Up @@ -200,7 +201,9 @@ export function nodeResolve(opts = {}) {
preserveSymlinks,
useBrowserOverrides,
baseDir,
moduleDirectories
moduleDirectories,
rootDir,
ignoreSideEffectsForRoot
});

const resolved =
Expand Down
16 changes: 12 additions & 4 deletions packages/node-resolve/src/resolveImportSpecifiers.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ async function resolveId({
preserveSymlinks,
useBrowserOverrides,
baseDir,
moduleDirectories
moduleDirectories,
rootDir,
ignoreSideEffectsForRoot
}) {
let hasModuleSideEffects = () => null;
let hasPackageEntry = true;
Expand All @@ -58,7 +60,9 @@ async function resolveId({
pkgPath,
mainFields,
preserveSymlinks,
useBrowserOverrides
useBrowserOverrides,
rootDir,
ignoreSideEffectsForRoot
});

({ packageInfo, hasModuleSideEffects, hasPackageEntry, packageBrowserField } = info);
Expand Down Expand Up @@ -180,7 +184,9 @@ export default async function resolveImportSpecifiers({
preserveSymlinks,
useBrowserOverrides,
baseDir,
moduleDirectories
moduleDirectories,
rootDir,
ignoreSideEffectsForRoot
}) {
let lastResolveError;

Expand All @@ -197,7 +203,9 @@ export default async function resolveImportSpecifiers({
preserveSymlinks,
useBrowserOverrides,
baseDir,
moduleDirectories
moduleDirectories,
rootDir,
ignoreSideEffectsForRoot
});

if (result instanceof ResolveError) {
Expand Down
27 changes: 19 additions & 8 deletions packages/node-resolve/src/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './side-effect.js';
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"main": "./index.js",
"sideEffects": false
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log('side effect');
17 changes: 17 additions & 0 deletions packages/node-resolve/test/snapshots/test.js.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Binary file modified packages/node-resolve/test/snapshots/test.js.snap
Binary file not shown.
31 changes: 31 additions & 0 deletions packages/node-resolve/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit 7359d1f

Please sign in to comment.