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

feat(node-resolve): add ignoreSideEffectsForRoot option #694

Merged
merged 6 commits into from
Feb 14, 2021
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
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