fix: remove isolated resolve() for compat with browser distribution (#3935)

* fix: remove isolated resolve()

This is incompatible with the browser implementation of the method.

* Set up basic browser tests

* Update test suites

* Update browser/error.ts

Co-authored-by: Craig Morten <cmorten@users.noreply.github.com>

* Use posix paths in test config

Co-authored-by: Lukas Taegert-Atkinson <lukas.taegert-atkinson@tngtech.com>
Co-authored-by: Lukas Taegert-Atkinson <lukastaegert@users.noreply.github.com>
pull/3943/head
Craig Morten 3 years ago committed by GitHub
parent 4519b11274
commit b99eea8e70
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -1,3 +0,0 @@
{
"spec": "test/test.js"
}

@ -1,4 +1,4 @@
{
"exclude": ["*commonjsHelpers.js", "test"],
"exclude": ["test"],
"extension": [".ts", ""]
}

@ -1,7 +1,4 @@
{
// Use IntelliSense to learn about possible attributes.
// Hover to view descriptions of existing attributes.
// For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
"version": "0.2.0",
"configurations": [
{

@ -1,4 +1,3 @@
// Place your settings in this file to overwrite default and user settings.
{
"typescript.format.insertSpaceBeforeFunctionParenthesis": true,
"typescript.format.insertSpaceAfterConstructor": true,

@ -0,0 +1,9 @@
import { error } from '../src/utils/error';
export const throwNoFileSystem = (method: string) => (..._args: any[]): never => {
error({
code: 'NO_FS_IN_BROWSER',
message: `Cannot access the file system (via "${method}") when using the browser build of Rollup. Make sure you supply a plugin with custom resolveId and load hooks to Rollup.`,
url: 'https://rollupjs.org/guide/en/#a-simple-example'
});
};

@ -1,14 +1,4 @@
const nope = (method: string) => (..._args: any[]): never => {
throw Object.assign(
new Error(
`Cannot access the file system (via "fs.${method}") when using the browser build of Rollup. Make sure you supply a plugin with custom resolveId and load hooks to Rollup.`
),
{ code: 'NO_FS_IN_BROWSER', url: 'https://rollupjs.org/guide/en/#a-simple-example' }
);
};
import { throwNoFileSystem } from './error';
export const lstatSync = nope('lstatSync');
export const readdirSync = nope('readdirSync');
export const readFile = nope('readFile');
export const realpathSync = nope('realpathSync');
export const writeFile = nope('writeFile');
export const readFile = throwNoFileSystem('fs.readFile');
export const writeFile = throwNoFileSystem('fs.writeFile');

@ -58,9 +58,13 @@ export function relative(from: string, to: string) {
}
export function resolve(...paths: string[]) {
let resolvedParts = paths.shift()!.split(/[/\\]/);
const firstPathSegment = paths.shift();
if (!firstPathSegment) {
return '/';
}
let resolvedParts = firstPathSegment.split(/[/\\]/);
paths.forEach(path => {
for (const path of paths) {
if (isAbsolute(path)) {
resolvedParts = path.split(/[/\\]/);
} else {
@ -75,7 +79,7 @@ export function resolve(...paths: string[]) {
resolvedParts.push.apply(resolvedParts, parts);
}
});
}
return resolvedParts.join('/');
}

@ -0,0 +1,23 @@
import { CustomPluginOptions } from '../src/rollup/types';
import { PluginDriver } from '../src/utils/PluginDriver';
import { throwNoFileSystem } from './error';
export async function resolveId(
source: string,
importer: string | undefined,
_preserveSymlinks: boolean,
pluginDriver: PluginDriver,
skip: number | null,
customOptions: CustomPluginOptions | undefined
) {
const pluginResult = await pluginDriver.hookFirst(
'resolveId',
[source, importer, { custom: customOptions }],
null,
skip
);
if (pluginResult == null) {
throwNoFileSystem('path.resolve');
}
return pluginResult;
}

@ -0,0 +1,27 @@
import path from 'path';
const ID_CRYPTO = path.resolve('src/utils/crypto');
const ID_FS = path.resolve('src/utils/fs');
const ID_PATH = path.resolve('src/utils/path');
const ID_RESOLVEID = path.resolve('src/utils/resolveId');
export default function replaceBrowserModules() {
return {
name: 'replace-browser-modules',
resolveId: (source, importee) => {
if (importee && source[0] === '.') {
const resolved = path.join(path.dirname(importee), source);
switch (resolved) {
case ID_CRYPTO:
return path.resolve('browser/crypto.ts');
case ID_FS:
return path.resolve('browser/fs.ts');
case ID_PATH:
return path.resolve('browser/path.ts');
case ID_RESOLVEID:
return path.resolve('browser/resolveId.ts');
}
}
}
};
}

@ -10,34 +10,34 @@
},
"scripts": {
"build": "shx rm -rf dist && git rev-parse HEAD > .commithash && rollup -c && shx cp src/rollup/types.d.ts dist/rollup.d.ts && shx chmod a+x dist/bin/rollup",
"build:test": "shx rm -rf dist && rollup -c --configTest && shx cp src/rollup/types.d.ts dist/rollup.d.ts && shx chmod a+x dist/bin/rollup",
"build:cjs": "shx rm -rf dist && rollup -c --configTest && shx cp src/rollup/types.d.ts dist/rollup.d.ts && shx chmod a+x dist/bin/rollup",
"build:bootstrap": "dist/bin/rollup -c && shx cp src/rollup/types.d.ts dist/rollup.d.ts && shx chmod a+x dist/bin/rollup",
"ci:lint": "npm run lint:nofix",
"ci:test": "npm run build:test && npm run build:bootstrap && npm run test:all",
"ci:test:only": "npm run build:test && npm run build:bootstrap && npm run test:only",
"ci:coverage": "npm run build:test && nyc --reporter lcovonly mocha && codecov",
"ci:test": "npm run build:cjs && npm run build:bootstrap && npm run test:all",
"ci:test:only": "npm run build:cjs && npm run build:bootstrap && npm run test:only",
"ci:coverage": "npm run build:cjs && nyc --reporter lcovonly mocha && codecov",
"lint": "npm run lint:ts -- --fix && npm run lint:js -- --fix && npm run lint:markdown",
"lint:nofix": "npm run lint:ts && npm run lint:js && npm run lint:markdown",
"lint:ts": "tslint --project .",
"lint:js": "eslint test/test.js test/*/index.js test/utils.js test/**/_config.js",
"lint:markdown": "markdownlint --config markdownlint.json docs/**/*.md",
"perf": "npm run build:test && node --expose-gc scripts/perf.js",
"perf": "npm run build:cjs && node --expose-gc scripts/perf.js",
"perf:debug": "node --inspect-brk scripts/perf-debug.js",
"perf:init": "node scripts/perf-init.js",
"prepare": "npm run build",
"prepublishOnly": "npm ci && npm run lint:nofix && npm run security && npm run build:bootstrap && npm run test:all",
"pretest": "npm run build:test",
"pretest:coverage": "npm run build:test && shx rm -rf coverage/*",
"pretest:typescript": "shx rm -rf test/typescript/dist && shx cp -r dist test/typescript/",
"security": "# npm audit # deactivated until there is a solution for the lodash issue",
"test": "npm run test:all",
"test:all": "npm run test:only && npm run test:typescript && npm run test:leak && npm run test:package",
"test:coverage": "nyc --reporter html mocha",
"security": "npm audit",
"test": "npm run build && npm run test:all",
"test:cjs": "npm run build:cjs && npm run test:only",
"test:quick": "mocha -b test/test.js",
"test:all": "npm run test:only && npm run test:browser && npm run test:typescript && npm run test:leak && npm run test:package",
"test:coverage": "npm run build:cjs && shx rm -rf coverage/* && nyc --reporter html mocha test/test.js",
"test:coverage:browser": "npm run build && shx rm -rf coverage/* && nyc mocha test/browser/index.js",
"test:leak": "node --expose-gc test/leak/index.js",
"test:package": "node scripts/test-package.js",
"test:only": "mocha",
"test:quick": "mocha -b",
"test:typescript": "tsc --noEmit -p test/typescript && tsc --noEmit",
"test:only": "mocha test/test.js",
"test:typescript": "shx rm -rf test/typescript/dist && shx cp -r dist test/typescript/ && tsc --noEmit -p test/typescript && tsc --noEmit",
"test:browser": "mocha test/browser/index.js",
"watch": "rollup -cw"
},
"repository": "rollup/rollup",

@ -12,6 +12,7 @@ import conditionalFsEventsImport from './build-plugins/conditional-fsevents-impo
import emitModulePackageFile from './build-plugins/emit-module-package-file.js';
import esmDynamicImport from './build-plugins/esm-dynamic-import.js';
import getLicenseHandler from './build-plugins/generate-license-file';
import replaceBrowserModules from './build-plugins/replace-browser-modules.js';
import pkg from './package.json';
const commitHash = (function () {
@ -142,16 +143,10 @@ export default command => {
input: 'src/browser-entry.ts',
onwarn,
plugins: [
replaceBrowserModules(),
alias(moduleAliases),
resolve({ browser: true }),
json(),
{
load: id => {
if (~id.indexOf('crypto.ts')) return fs.readFileSync('browser/crypto.ts', 'utf-8');
if (~id.indexOf('fs.ts')) return fs.readFileSync('browser/fs.ts', 'utf-8');
if (~id.indexOf('path.ts')) return fs.readFileSync('browser/path.ts', 'utf-8');
}
},
commonjs(),
typescript(),
terser({ module: true, output: { comments: 'some' } }),
@ -161,7 +156,7 @@ export default command => {
treeshake,
strictDeprecations: true,
output: [
{ file: 'dist/rollup.browser.js', format: 'umd', name: 'rollup', banner },
{ file: 'dist/rollup.browser.js', format: 'umd', name: 'rollup', banner, sourcemap: true },
{ file: 'dist/es/rollup.browser.js', format: 'es', banner }
]
};

@ -1,4 +1,4 @@
import { basename, extname, isAbsolute, relative } from './path';
import { basename, extname, isAbsolute, relative, resolve } from './path';
import { sanitizeFileName } from './sanitizeFileName';
export function getAliasName(id: string) {
@ -7,8 +7,8 @@ export function getAliasName(id: string) {
}
export default function relativeId(id: string) {
if (typeof process === 'undefined' || !isAbsolute(id)) return id;
return relative(process.cwd(), id);
if (!isAbsolute(id)) return id;
return relative(resolve(), id);
}
export function isPlainPathFragment(name: string) {

@ -28,7 +28,7 @@ export async function resolveId(
// resolve call and require no special handing on our part.
// See https://nodejs.org/api/path.html#path_path_resolve_paths
return addJsExtensionIfNecessary(
resolve(importer ? dirname(importer) : resolve(), source),
importer ? resolve(dirname(importer), source) : resolve(source),
preserveSymlinks
);
}

@ -0,0 +1,76 @@
const fixturify = require('fixturify');
const { basename, resolve } = require('path');
const { rollup } = require('../../dist/rollup.browser.js');
const { assertFilesAreEqual, runTestSuiteWithSamples, compareError } = require('../utils.js');
runTestSuiteWithSamples('browser', resolve(__dirname, 'samples'), (dir, config) => {
(config.skip ? it.skip : config.solo ? it.only : it)(
basename(dir) + ': ' + config.description,
async () => {
let bundle;
try {
bundle = await rollup({
input: 'main',
onwarn: warning => {
if (!(config.expectedWarnings && config.expectedWarnings.indexOf(warning.code) >= 0)) {
throw new Error(
`Unexpected warnings (${warning.code}): ${warning.message}\n` +
'If you expect warnings, list their codes in config.expectedWarnings'
);
}
},
strictDeprecations: true,
...config.options
});
} catch (error) {
if (config.error) {
compareError(error, config.error);
return;
} else {
throw error;
}
}
if (config.error) {
throw new Error('Expected an error while rolling up');
}
let output;
try {
({ output } = await bundle.generate({
exports: 'auto',
format: 'es',
...(config.options || {}).output
}));
} catch (error) {
if (config.generateError) {
compareError(error, config.generateError);
return;
} else {
throw error;
}
}
if (config.generateError) {
throw new Error('Expected an error while generating output');
}
assertOutputMatches(output, dir);
}
);
});
function assertOutputMatches(output, dir) {
const actual = {};
for (const file of output) {
const filePath = file.fileName.split('/');
const fileName = filePath.pop();
let currentDir = actual;
for (const pathElement of filePath) {
if (!currentDir[pathElement]) {
currentDir[pathElement] = {};
}
currentDir = currentDir[pathElement] = currentDir[pathElement] || {};
}
currentDir[fileName] = file.source || file.code;
}
fixturify.writeSync(resolve(dir, '_actual'), actual);
const expected = fixturify.readSync(resolve(dir, '_expected'));
assertFilesAreEqual(actual, expected);
}

@ -0,0 +1,12 @@
const { loader } = require('../../../utils.js');
module.exports = {
description: 'bundles files for the browser',
options: {
plugins: loader({
main: `import {foo} from 'dep';
console.log(foo);`,
dep: `export const foo = 42;`
})
}
};

@ -0,0 +1,3 @@
const foo = 42;
console.log(foo);

@ -0,0 +1,16 @@
const { loader } = require('../../../utils.js');
module.exports = {
description: 'fails if a dependency cannot be resolved',
options: {
plugins: loader({
main: `import {foo} from 'dep';
console.log(foo);`
})
},
error: {
message:
"Unexpected warnings (UNRESOLVED_IMPORT): 'dep' is imported by main, but could not be resolved treating it as an external dependency\nIf you expect warnings, list their codes in config.expectedWarnings",
watchFiles: ['main']
}
};

@ -0,0 +1,7 @@
module.exports = {
description: 'fails if an entry cannot be resolved',
error: {
code: 'UNRESOLVED_ENTRY',
message: 'Could not resolve entry module (main).'
}
};

@ -0,0 +1,17 @@
module.exports = {
description: 'fails if a file cannot be loaded',
options: {
plugins: {
resolveId(source) {
return source;
}
}
},
error: {
code: 'NO_FS_IN_BROWSER',
message:
'Could not load main: Cannot access the file system (via "fs.readFile") when using the browser build of Rollup. Make sure you supply a plugin with custom resolveId and load hooks to Rollup.',
url: 'https://rollupjs.org/guide/en/#a-simple-example',
watchFiles: ['main']
}
};

@ -0,0 +1,42 @@
const { join, dirname } = require('path').posix;
module.exports = {
description: 'renormalizes external paths if possible',
options: {
input: ['/main.js', '/nested/entry.js'],
external(id) {
return id.endsWith('ext');
},
plugins: {
resolveId(source, importer) {
if (source.endsWith('ext.js')) {
return false;
}
if (!importer) {
return source;
}
return join(dirname(importer), source);
},
load(id) {
switch (id) {
case '/main.js':
return `import './nested/dep.js';
import './ext.js';
import './nested/nested-ext.js';`;
case '/dep.js':
return `import './ext.js';
import './nested/nested-ext.js';`;
case '/nested/dep.js':
return `import '../ext.js';
import './nested-ext.js';`;
case '/nested/entry.js':
return `import '../dep.js';
import '../ext.js';
import './nested-ext.js';`;
default:
throw new Error(`Unexpected id ${id}`);
}
}
}
}
};

@ -0,0 +1,2 @@
import './ext.js';
import './nested/nested-ext.js';

@ -0,0 +1,2 @@
import './ext.js';
import './nested/nested-ext.js';

@ -0,0 +1,16 @@
const { loader } = require('../../../utils.js');
module.exports = {
description: 'bundles files for the browser',
options: {
input: ['main', 'dep'],
plugins: loader({
main: `import {foo} from 'dep';
console.log(foo);`,
dep: `export const foo = 42;`
}),
output: {
entryFileNames: '[name]-[hash].js'
}
}
};

@ -0,0 +1,3 @@
import { foo } from './dep-cf8755fa.js';
console.log(foo);

@ -12,6 +12,7 @@ exports.loader = loader;
exports.normaliseOutput = normaliseOutput;
exports.runTestSuiteWithSamples = runTestSuiteWithSamples;
exports.assertDirectoriesAreEqual = assertDirectoriesAreEqual;
exports.assertFilesAreEqual = assertFilesAreEqual;
exports.assertIncludes = assertIncludes;
function normaliseError(error) {

Loading…
Cancel
Save