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: remove isolated resolve() for compat with browser distribution #3935

Merged
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
3 changes: 0 additions & 3 deletions .mocharc.json

This file was deleted.

2 changes: 1 addition & 1 deletion .nycrc
@@ -1,4 +1,4 @@
{
"exclude": ["*commonjsHelpers.js", "test"],
"exclude": ["test"],
"extension": [".ts", ""]
}
3 changes: 0 additions & 3 deletions .vscode/launch.json
@@ -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": [
{
Expand Down
1 change: 0 additions & 1 deletion .vscode/settings.json
@@ -1,4 +1,3 @@
// Place your settings in this file to overwrite default and user settings.
{
"typescript.format.insertSpaceBeforeFunctionParenthesis": true,
"typescript.format.insertSpaceAfterConstructor": true,
Expand Down
9 changes: 9 additions & 0 deletions browser/error.ts
@@ -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'
});
};
16 changes: 3 additions & 13 deletions browser/fs.ts
@@ -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');
10 changes: 7 additions & 3 deletions browser/path.ts
Expand Up @@ -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) {
cmorten marked this conversation as resolved.
Show resolved Hide resolved
return '/';
}
let resolvedParts = firstPathSegment.split(/[/\\]/);

paths.forEach(path => {
for (const path of paths) {
if (isAbsolute(path)) {
resolvedParts = path.split(/[/\\]/);
} else {
Expand All @@ -75,7 +79,7 @@ export function resolve(...paths: string[]) {

resolvedParts.push.apply(resolvedParts, parts);
}
});
}

return resolvedParts.join('/');
}
23 changes: 23 additions & 0 deletions browser/resolveId.ts
@@ -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;
}
27 changes: 27 additions & 0 deletions build-plugins/replace-browser-modules.js
@@ -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');
}
}
}
};
}
30 changes: 15 additions & 15 deletions package.json
Expand Up @@ -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",
Expand Down
11 changes: 3 additions & 8 deletions rollup.config.js
Expand Up @@ -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 () {
Expand Down Expand Up @@ -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' } }),
Expand All @@ -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 }
]
};
Expand Down
6 changes: 3 additions & 3 deletions src/utils/relativeId.ts
@@ -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) {
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/utils/resolveId.ts
Expand Up @@ -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
);
}
Expand Down
76 changes: 76 additions & 0 deletions test/browser/index.js
@@ -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);
}
12 changes: 12 additions & 0 deletions test/browser/samples/basic/_config.js
@@ -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;`
})
}
};
3 changes: 3 additions & 0 deletions test/browser/samples/basic/_expected/main.js
@@ -0,0 +1,3 @@
const foo = 42;

console.log(foo);
16 changes: 16 additions & 0 deletions test/browser/samples/missing-dependency-resolution/_config.js
@@ -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']
}
};
7 changes: 7 additions & 0 deletions test/browser/samples/missing-entry-resolution/_config.js
@@ -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).'
}
};
17 changes: 17 additions & 0 deletions test/browser/samples/missing-load/_config.js
@@ -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']
}
};