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): prepare for Rollup 3 #1288

Merged
merged 1 commit into from Oct 10, 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
2 changes: 1 addition & 1 deletion packages/node-resolve/README.md
Expand Up @@ -13,7 +13,7 @@

## Requirements

This plugin requires an [LTS](https://github.com/nodejs/Release) Node version (v8.0.0+) and Rollup v1.20.0+.
This plugin requires an [LTS](https://github.com/nodejs/Release) Node version (v14.0.0+) and Rollup v2.78.0+.

## Install

Expand Down
38 changes: 21 additions & 17 deletions packages/node-resolve/package.json
Expand Up @@ -15,13 +15,13 @@
"bugs": "https://github.com/rollup/plugins/issues",
"main": "./dist/cjs/index.js",
"module": "./dist/es/index.js",
"type": "commonjs",
"exports": {
"require": "./dist/cjs/index.js",
"import": "./dist/es/index.js"
"types": "./types/index.d.ts",
"import": "./dist/es/index.js",
"default": "./dist/cjs/index.js"
},
"engines": {
"node": ">= 10.0.0"
"node": ">=14.0.0"
},
"scripts": {
"build": "rollup -c",
Expand All @@ -40,6 +40,7 @@
},
"files": [
"dist",
"!dist/**/*.map",
"types",
"README.md",
"LICENSE"
Expand All @@ -52,32 +53,35 @@
"modules"
],
"peerDependencies": {
"rollup": "^2.78.0"
"rollup": "^2.78.0||^3.0.0"
},
"peerDependenciesMeta": {
"rollup": {
"optional": true
}
},
"dependencies": {
"@rollup/pluginutils": "^3.1.0",
"@types/resolve": "1.17.1",
"@rollup/pluginutils": "^4.2.1",
"@types/resolve": "1.20.2",
"deepmerge": "^4.2.2",
"is-builtin-module": "^3.1.0",
"is-builtin-module": "^3.2.0",
"is-module": "^1.0.0",
"resolve": "^1.19.0"
"resolve": "^1.22.1"
},
"devDependencies": {
"@babel/core": "^7.10.5",
"@babel/core": "^7.19.1",
"@babel/plugin-transform-typescript": "^7.10.5",
"@rollup/plugin-babel": "^5.1.0",
"@rollup/plugin-commonjs": "^22.0.2",
"@rollup/plugin-json": "^4.1.0",
"es5-ext": "^0.10.53",
"rollup": "^2.78.1",
"source-map": "^0.7.3",
"es5-ext": "^0.10.62",
"rollup": "^3.0.0-7",
"source-map": "^0.7.4",
"string-capitalize": "^1.0.1"
},
"types": "types/index.d.ts",
"types": "./types/index.d.ts",
"ava": {
"babel": {
"compileEnhancements": false
},
"workerThreads": false,
"files": [
"!**/fixtures/**",
"!**/helpers/**",
Expand Down
18 changes: 0 additions & 18 deletions packages/node-resolve/rollup.config.js

This file was deleted.

13 changes: 13 additions & 0 deletions packages/node-resolve/rollup.config.mjs
@@ -0,0 +1,13 @@
import { readFileSync } from 'fs';

import json from '@rollup/plugin-json';

import { createConfig } from '../../shared/rollup.config.mjs';

export default {
...createConfig({
pkg: JSON.parse(readFileSync(new URL('./package.json', import.meta.url), 'utf8'))
}),
input: 'src/index.js',
plugins: [json()]
};
4 changes: 3 additions & 1 deletion packages/node-resolve/src/index.js
Expand Up @@ -5,12 +5,13 @@ import isBuiltinModule from 'is-builtin-module';
import deepMerge from 'deepmerge';
import isModule from 'is-module';

import { version } from '../package.json';
import { version, peerDependencies } from '../package.json';

import { isDirCached, isFileCached, readCachedFile } from './cache';
import handleDeprecatedOptions from './deprecated-options';
import { fileExists, readFile, realpath } from './fs';
import resolveImportSpecifiers from './resolveImportSpecifiers';
import validateVersion from './rollup-version.js';
import { getMainFields, getPackageName, normalizeInput } from './util';

const ES6_BROWSER_EMPTY = '\0node-resolve:empty.js';
Expand Down Expand Up @@ -249,6 +250,7 @@ export function nodeResolve(opts = {}) {
version,

buildStart(buildOptions) {
validateVersion(this.meta.rollupVersion, peerDependencies.rollup);
rollupOptions = buildOptions;

for (const warning of warnings) {
Expand Down
31 changes: 31 additions & 0 deletions packages/node-resolve/src/rollup-version.js
@@ -0,0 +1,31 @@
const versionRegexp = /\^(\d+\.\d+\.\d+)/g;

export default function validateVersion(actualVersion, peerDependencyVersion) {
let minMajor = Infinity;
let minMinor = Infinity;
let minPatch = Infinity;
let foundVersion;
// eslint-disable-next-line no-cond-assign
while ((foundVersion = versionRegexp.exec(peerDependencyVersion))) {
const [foundMajor, foundMinor, foundPatch] = foundVersion[1].split('.').map(Number);
if (foundMajor < minMajor) {
minMajor = foundMajor;
minMinor = foundMinor;
minPatch = foundPatch;
}
}
if (!actualVersion) {
throw new Error(
`Insufficient Rollup version: "@rollup/plugin-node-resolve" requires at least rollup@${minMajor}.${minMinor}.${minPatch}.`
);
}
const [major, minor, patch] = actualVersion.split('.').map(Number);
if (
major < minMajor ||
(major === minMajor && (minor < minMinor || (minor === minMinor && patch < minPatch)))
) {
throw new Error(
`Insufficient rollup version: "@rollup/plugin-node-resolve" requires at least rollup@${minMajor}.${minMinor}.${minPatch} but found rollup@${actualVersion}.`
);
}
}
8 changes: 5 additions & 3 deletions packages/node-resolve/test/jail.js
Expand Up @@ -22,10 +22,12 @@ test('mark module outside the jail as external', async (t) => {
]
});
const imports = await getImports(bundle);

t.snapshot(warnings);
t.is(warnings.length, 1);
t.deepEqual(imports, ['string/uppercase.js']);

t.is(warnings.length, 1, 'number of warnings');
const [{ exporter, id }] = warnings;
t.is(exporter, 'string/uppercase.js', 'exporter');
t.is(id.endsWith('jail.js'), true, 'id');
});

test('bundle module defined inside the jail', async (t) => {
Expand Down
1 change: 1 addition & 0 deletions packages/node-resolve/test/node_modules/current-package

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

@@ -1,14 +1,16 @@
import { join } from 'path';
import { fileURLToPath } from 'url';

import test from 'ava';
import { rollup } from 'rollup';

import commonjs from '@rollup/plugin-commonjs';

import { nodeResolve } from '..';
import { getCode, testBundle } from '../../../util/test';
import { nodeResolve } from 'current-package';

import { getCode, testBundle } from '../../../util/test.js';

process.chdir(join(__dirname, 'fixtures'));
const DIRNAME = fileURLToPath(new URL('.', import.meta.url));
process.chdir(join(DIRNAME, 'fixtures'));

const failOnWarn = (t) => (warning) =>
t.fail(`No warnings were expected, got:\n${warning.code}\n${warning.message}`);
Expand Down
Binary file modified packages/node-resolve/test/snapshots/dedupe-custom.js.snap
Binary file not shown.
Binary file modified packages/node-resolve/test/snapshots/dedupe.js.snap
Binary file not shown.
20 changes: 0 additions & 20 deletions packages/node-resolve/test/snapshots/jail.js.md

This file was deleted.

Binary file removed packages/node-resolve/test/snapshots/jail.js.snap
Binary file not shown.
Binary file modified packages/node-resolve/test/snapshots/only.js.snap
Binary file not shown.
6 changes: 4 additions & 2 deletions packages/node-resolve/test/snapshots/prefer-builtins.js.md
Expand Up @@ -10,9 +10,11 @@ Generated by [AVA](https://avajs.dev).

[
{
chunkName: 'prefer-builtin',
code: 'EMPTY_BUNDLE',
message: 'Generated an empty chunk: "prefer-builtin"',
message: 'Generated an empty chunk: "prefer-builtin".',
names: [
'prefer-builtin',
],
toString: Function {},
},
]
Binary file modified packages/node-resolve/test/snapshots/prefer-builtins.js.snap
Binary file not shown.
Binary file modified packages/node-resolve/test/snapshots/root-dir.js.snap
Binary file not shown.
Binary file not shown.
@@ -1,6 +1,6 @@
# Snapshot report for `test/side-effects.js`
# Snapshot report for `test/side-effects.mjs`

The actual snapshot is saved in `side-effects.js.snap`.
The actual snapshot is saved in `side-effects.mjs.snap`.

Generated by [AVA](https://avajs.dev).

Expand Down
Binary file not shown.
Binary file removed packages/node-resolve/test/snapshots/test.js.snap
Binary file not shown.
@@ -1,14 +1,14 @@
# Snapshot report for `test/test.js`
# Snapshot report for `test/test.mjs`

The actual snapshot is saved in `test.js.snap`.
The actual snapshot is saved in `test.mjs.snap`.

Generated by [AVA](https://avajs.dev).

## throws error if local id is not resolved

> Snapshot 1

'Could not resolve \'./foo\' from unresolved-local.js'
'Could not resolve "./foo" from "unresolved-local.js"'

## allows custom moduleDirectories with legacy customResolveOptions.moduleDirectory

Expand All @@ -23,56 +23,6 @@ Generated by [AVA](https://avajs.dev).
},
]

## ignores deep-import non-modules

> Snapshot 1

[
{
code: 'UNRESOLVED_IMPORT',
importer: 'deep-import-non-module.js',
message: '\'foo/deep\' is imported by deep-import-non-module.js, but could not be resolved – treating it as an external dependency',
source: 'foo/deep',
toString: Function {},
url: 'https://rollupjs.org/guide/en/#warning-treating-module-as-external-dependency',
},
]

## respects the package.json sideEffects property for files in root package by default

> Snapshot 1

`'use strict';␊
console.log('main');␊
`

## ignores the package.json sideEffects property for files in root package with "ignoreSideEffectsForRoot" option

> Snapshot 1

`'use strict';␊
console.log('side effect');␊
console.log('main');␊
`

## handles package side-effects

> Snapshot 1

[
'array-dep1',
'array-dep3',
'array-dep5',
'array-index',
'false-dep1',
'true-dep1',
'true-dep2',
'true-index',
]

## throws error for removed customResolveOptions.preserveSymlinks option

> Snapshot 1
Expand Down Expand Up @@ -176,27 +126,3 @@ 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 added packages/node-resolve/test/snapshots/test.mjs.snap
Binary file not shown.