Skip to content

Commit

Permalink
[v3.0] Refine errors and warnings (#4579)
Browse files Browse the repository at this point in the history
* [v3.0] New hashing algorithm that "fixes (nearly) everything" (#4543)

* Initial new hashing idea

* Simplify external import path generation

197 broken tests left

* Use correct file names in chunk info

197 broken tests left

* Implement first draft for hashing algorithm

189 broken tests left

* Remove active deprecations

this.emitAsset
this.emitChunk
this.getAssetFileName
this.getChunkFileName
import.meta.ROLLUP_ASSET_URL_
import.meta.ROLLUP_CHUNK_URL_

* Reduce render parameters

* Always scan all chunks for hashes

* Fix asset emission and remaining tests

* Reintroduce augmentChunkHash and get OutputChunk by converting RenderedChunk

* Provide chunk graph in renderChunk

* Handle hash collisions

* Remove deprecated hacky asset emission

* Allow to configure hash sizes per file

* Update documentation

* Extend tests

* Minor improvements

* Improve documentation about hashing

* Replace hash in sourcemap file

* Provide ChunkInfo in banner/footer/intro/outro

* Extract hashing logic

* Clean up hashing logic

* Add ExternalChunk wrapper

* Store inputBase on Chunk

* Store snippets on Chunk

* Align chunk interfaces

* Reduce this. property access

* Move dynamicImportFunction warning to options normalization

* Restructure rendering logic

* Do not run on Node 10

* Update documentation

* Try to fix Windows tests

* Improve coverage

* Remove graph background colors

3.0.0-0

* [v3.0] Change default for output.generatedCode.reservedNamesAsProps (#4568)

* [v3.0] New hashing algorithm that "fixes (nearly) everything" (#4543)

* Initial new hashing idea

* Simplify external import path generation

197 broken tests left

* Use correct file names in chunk info

197 broken tests left

* Implement first draft for hashing algorithm

189 broken tests left

* Remove active deprecations

this.emitAsset
this.emitChunk
this.getAssetFileName
this.getChunkFileName
import.meta.ROLLUP_ASSET_URL_
import.meta.ROLLUP_CHUNK_URL_

* Reduce render parameters

* Always scan all chunks for hashes

* Fix asset emission and remaining tests

* Reintroduce augmentChunkHash and get OutputChunk by converting RenderedChunk

* Provide chunk graph in renderChunk

* Handle hash collisions

* Remove deprecated hacky asset emission

* Allow to configure hash sizes per file

* Update documentation

* Extend tests

* Minor improvements

* Improve documentation about hashing

* Replace hash in sourcemap file

* Provide ChunkInfo in banner/footer/intro/outro

* Extract hashing logic

* Clean up hashing logic

* Add ExternalChunk wrapper

* Store inputBase on Chunk

* Store snippets on Chunk

* Align chunk interfaces

* Reduce this. property access

* Move dynamicImportFunction warning to options normalization

* Restructure rendering logic

* Do not run on Node 10

* Update documentation

* Try to fix Windows tests

* Improve coverage

* Remove graph background colors

3.0.0-0

* Rework warnings and errors

* Refine some error messages
* Reduce number of different props of errors
* All errors are declared in error.ts
* Use name RollupError for errors that do not have a cause

* Extend documentation
  • Loading branch information
lukastaegert committed Sep 6, 2022
1 parent 9a8efa5 commit 7a78648
Show file tree
Hide file tree
Showing 155 changed files with 1,285 additions and 1,008 deletions.
11 changes: 3 additions & 8 deletions browser/error.ts
@@ -1,9 +1,4 @@
import { error } from '../src/utils/error';
import { errNoFileSystemInBrowser, error } from '../src/utils/error';

export const throwNoFileSystem = (method: string) => (): 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'
});
};
export const throwNoFileSystem = (method: string) => (): never =>
error(errNoFileSystemInBrowser(method));
7 changes: 4 additions & 3 deletions cli/logging.ts
Expand Up @@ -7,9 +7,10 @@ import relativeId from '../src/utils/relativeId';
export const stderr = (...args: readonly unknown[]) => process.stderr.write(`${args.join('')}\n`);

export function handleError(err: RollupError, recover = false): void {
let description = err.message || err;
if (err.name) description = `${err.name}: ${description}`;
const message = (err.plugin ? `(plugin ${err.plugin}) ${description}` : description) || err;
const name = err.name || err.cause?.name;
const nameSection = name ? `${name}: ` : '';
const pluginSection = err.plugin ? `(plugin ${err.plugin}) ` : '';
const message = `${pluginSection}${nameSection}${err.message}`;

stderr(bold(red(`[!] ${bold(message.toString())}`)));

Expand Down
47 changes: 23 additions & 24 deletions cli/run/batchWarnings.ts
Expand Up @@ -31,7 +31,7 @@ export default function batchWarnings(): BatchWarnings {

if (warning.url) info(warning.url);

const id = (warning.loc && warning.loc.file) || warning.id;
const id = warning.loc?.file || warning.id;
if (id) {
const loc = warning.loc
? `${relativeId(id)} (${warning.loc.line}:${warning.loc.column})`
Expand Down Expand Up @@ -77,7 +77,7 @@ const immediateHandlers: {

stderr(
`Creating a browser bundle that depends on ${printQuotedStringList(
warning.modules!
warning.ids!
)}. You might need to include https://github.com/FredKSchott/rollup-plugin-polyfill-node`
);
},
Expand All @@ -95,7 +95,7 @@ const deferredHandlers: {
title(`Circular dependenc${warnings.length > 1 ? 'ies' : 'y'}`);
const displayed = warnings.length > 5 ? warnings.slice(0, 3) : warnings;
for (const warning of displayed) {
stderr(warning.cycle!.join(' -> '));
stderr(warning.ids!.map(relativeId).join(' -> '));
}
if (warnings.length > displayed.length) {
stderr(`...and ${warnings.length - displayed.length} more`);
Expand All @@ -108,7 +108,7 @@ const deferredHandlers: {
warnings.length > 1 ? 'chunks' : 'chunk'
}`
);
stderr(warnings.map(warning => warning.chunkName!).join(', '));
stderr(printQuotedStringList(warnings.map(warning => warning.names![0])));
},

EVAL(warnings) {
Expand All @@ -122,19 +122,20 @@ const deferredHandlers: {
info('https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module');

for (const warning of warnings) {
stderr(bold(warning.importer!));
stderr(`${warning.missing} is not exported by ${warning.exporter}`);
stderr(bold(relativeId(warning.id!)));
stderr(`${warning.binding} is not exported by ${relativeId(warning.exporter!)}`);
stderr(gray(warning.frame!));
}
},

MISSING_GLOBAL_NAME(warnings) {
title(`Missing global variable ${warnings.length > 1 ? 'names' : 'name'}`);
info('https://rollupjs.org/guide/en/#outputglobals');
stderr(
`Use output.globals to specify browser global variable names corresponding to external modules`
`Use "output.globals" to specify browser global variable names corresponding to external modules:`
);
for (const warning of warnings) {
stderr(`${bold(warning.source!)} (guessing '${warning.guess}')`);
stderr(`${bold(warning.id!)} (guessing "${warning.names![0]}")`);
}
},

Expand All @@ -151,7 +152,7 @@ const deferredHandlers: {
stderr(`...and ${warnings.length - displayedWarnings.length} other entry modules`);
}
stderr(
`\nConsumers of your bundle will have to use chunk['default'] to access their default export, which may not be what you want. Use \`output.exports: 'named'\` to disable this warning`
`\nConsumers of your bundle will have to use chunk.default to access their default export, which may not be what you want. Use \`output.exports: "named"\` to disable this warning.`
);
},

Expand All @@ -160,19 +161,14 @@ const deferredHandlers: {
for (const warning of warnings) {
stderr(
`"${bold(relativeId(warning.reexporter!))}" re-exports "${
warning.name
}" from both "${relativeId(warning.sources![0])}" and "${relativeId(
warning.sources![1]
)}" (will be ignored)`
warning.binding
}" from both "${relativeId(warning.ids![0])}" and "${relativeId(
warning.ids![1]
)}" (will be ignored).`
);
}
},

NON_EXISTENT_EXPORT(warnings) {
title(`Import of non-existent ${warnings.length > 1 ? 'exports' : 'export'}`);
showTruncatedWarnings(warnings);
},

PLUGIN_WARNING(warnings) {
const nestedByPlugin = nest(warnings, 'plugin');

Expand Down Expand Up @@ -208,12 +204,12 @@ const deferredHandlers: {
stderr(
`Plugins that transform code (such as ${printQuotedStringList(
plugins
)}) should generate accompanying sourcemaps`
)}) should generate accompanying sourcemaps.`
);
},

THIS_IS_UNDEFINED(warnings) {
title('`this` has been rewritten to `undefined`');
title('"this" has been rewritten to "undefined"');
info('https://rollupjs.org/guide/en/#error-this-is-undefined');
showTruncatedWarnings(warnings);
},
Expand All @@ -224,11 +220,13 @@ const deferredHandlers: {

const dependencies = new Map<string, string[]>();
for (const warning of warnings) {
getOrCreate(dependencies, warning.source, () => []).push(warning.importer!);
getOrCreate(dependencies, relativeId(warning.exporter!), () => []).push(
relativeId(warning.id!)
);
}

for (const [dependency, importers] of dependencies) {
stderr(`${bold(dependency)} (imported by ${importers.join(', ')})`);
stderr(`${bold(dependency)} (imported by ${printQuotedStringList(importers)})`);
}
},

Expand All @@ -238,9 +236,10 @@ const deferredHandlers: {
stderr(
warning.names +
' imported from external module "' +
warning.source +
warning.exporter +
'" but never used in ' +
printQuotedStringList(warning.sources!.map(id => relativeId(id)))
printQuotedStringList(warning.ids!.map(relativeId)) +
'.'
);
}
}
Expand Down
6 changes: 2 additions & 4 deletions cli/run/build.ts
Expand Up @@ -3,6 +3,7 @@ import ms from 'pretty-ms';
import { rollup } from '../../src/node-entry';
import type { MergedRollupOptions } from '../../src/rollup/types';
import { bold, cyan, green } from '../../src/utils/colors';
import { errOnlyInlineSourcemapsForStdout } from '../../src/utils/error';
import relativeId from '../../src/utils/relativeId';
import { SOURCEMAPPING_URL } from '../../src/utils/sourceMappingURL';
import { handleError, stderr } from '../logging';
Expand Down Expand Up @@ -34,10 +35,7 @@ export default async function build(
if (useStdout) {
const output = outputOptions[0];
if (output.sourcemap && output.sourcemap !== 'inline') {
handleError({
code: 'ONLY_INLINE_SOURCEMAPS',
message: 'Only inline sourcemaps are supported when bundling to stdout.'
});
handleError(errOnlyInlineSourcemapsForStdout());
}

const { output: outputs } = await bundle.generate(output);
Expand Down
6 changes: 2 additions & 4 deletions cli/run/getConfigPath.ts
@@ -1,6 +1,7 @@
import { promises as fs } from 'fs';
import { resolve } from 'path';
import { cwd } from 'process';
import { errMissingExternalConfig } from '../../src/utils/error';
import { handleError } from '../logging';

const DEFAULT_CONFIG_BASE = 'rollup.config';
Expand All @@ -18,10 +19,7 @@ export async function getConfigPath(commandConfig: string | true): Promise<strin
return require.resolve(pkgName, { paths: [cwd()] });
} catch (err: any) {
if (err.code === 'MODULE_NOT_FOUND') {
handleError({
code: 'MISSING_EXTERNAL_CONFIG',
message: `Could not resolve config file "${commandConfig}"`
});
handleError(errMissingExternalConfig(commandConfig));
}
throw err;
}
Expand Down
11 changes: 3 additions & 8 deletions cli/run/index.ts
@@ -1,5 +1,6 @@
import { env } from 'process';
import type { MergedRollupOptions } from '../../src/rollup/types';
import { errDuplicateImportOptions, errFailAfterWarnings } from '../../src/utils/error';
import { isWatchEnabled } from '../../src/utils/options/mergeOptions';
import { getAliasName } from '../../src/utils/relativeId';
import { loadFsEvents } from '../../src/watch/fsevents-importer';
Expand All @@ -14,10 +15,7 @@ export default async function runRollup(command: Record<string, any>): Promise<v
let inputSource;
if (command._.length > 0) {
if (command.input) {
handleError({
code: 'DUPLICATE_IMPORT_OPTIONS',
message: 'Either use --input, or pass input path as argument'
});
handleError(errDuplicateImportOptions());
}
inputSource = command._;
} else if (typeof command.input === 'string') {
Expand Down Expand Up @@ -67,10 +65,7 @@ export default async function runRollup(command: Record<string, any>): Promise<v
}
if (command.failAfterWarnings && warnings.warningOccurred) {
warnings.flush();
handleError({
code: 'FAIL_AFTER_WARNINGS',
message: 'Warnings occurred and --failAfterWarnings flag present'
});
handleError(errFailAfterWarnings());
}
} catch (err: any) {
warnings.flush();
Expand Down
16 changes: 3 additions & 13 deletions cli/run/loadConfigFile.ts
Expand Up @@ -4,7 +4,7 @@ import getPackageType from 'get-package-type';
import * as rollup from '../../src/node-entry';
import type { MergedRollupOptions } from '../../src/rollup/types';
import { bold } from '../../src/utils/colors';
import { error } from '../../src/utils/error';
import { errMissingConfig, error, errTranspiledEsmConfig } from '../../src/utils/error';
import { mergeOptions } from '../../src/utils/options/mergeOptions';
import type { GenericConfigObject } from '../../src/utils/options/options';
import relativeId from '../../src/utils/relativeId';
Expand Down Expand Up @@ -117,13 +117,7 @@ function loadConfigFromBundledFile(fileName: string, bundledCode: string): unkno
return config;
} catch (err: any) {
if (err.code === 'ERR_REQUIRE_ESM') {
return error({
code: 'TRANSPILED_ESM_CONFIG',
message: `While loading the Rollup configuration from "${relativeId(
fileName
)}", Node tried to require an ES module from a CommonJS file, which is not supported. A common cause is if there is a package.json file with "type": "module" in the same folder. You can try to fix this by changing the extension of your configuration file to ".cjs" or ".mjs" depending on the content, which will prevent Rollup from trying to preprocess the file but rather hand it to Node directly.`,
url: 'https://rollupjs.org/guide/en/#using-untranspiled-config-files'
});
return error(errTranspiledEsmConfig(fileName));
}
throw err;
}
Expand All @@ -134,11 +128,7 @@ async function getConfigList(configFileExport: any, commandOptions: any): Promis
? configFileExport(commandOptions)
: configFileExport);
if (Object.keys(config).length === 0) {
return error({
code: 'MISSING_CONFIG',
message: 'Config file must export an options object, or an array of options objects',
url: 'https://rollupjs.org/guide/en/#configuration-files'
});
return error(errMissingConfig());
}
return Array.isArray(config) ? config : [config];
}
2 changes: 2 additions & 0 deletions docs/02-javascript-api.md
Expand Up @@ -12,6 +12,8 @@ On a `bundle` object, you can call `bundle.generate` multiple times with differe

Once you're finished with the `bundle` object, you should call `bundle.close()`, which will let plugins clean up their external processes or services via the [`closeBundle`](guide/en/#closebundle) hook.

If an error occurs at either stage, it will return a Promise rejected with an Error, which you can identify via their `code` property. Besides `code` and `message`, many errors have additional properties you can use for custom reporting, see [`utils/error.ts`](https://github.com/rollup/rollup/blob/master/src/utils/error.ts) for a complete list of errors and warnings together with their codes and properties.

```javascript
import { rollup } from 'rollup';

Expand Down
6 changes: 4 additions & 2 deletions docs/999-big-list-of-options.md
Expand Up @@ -370,7 +370,7 @@ Type: `(warning: RollupWarning, defaultHandler: (warning: string | RollupWarning
A function that will intercept warning messages. If not supplied, warnings will be deduplicated and printed to the console. When using the [`--silent`](guide/en/#--silent) CLI option, this handler is the only way to get notified about warnings.
The function receives two arguments: the warning object and the default handler. Warnings objects have, at a minimum, a `code` and a `message` property, allowing you to control how different kinds of warnings are handled. Other properties are added depending on the type of warning.
The function receives two arguments: the warning object and the default handler. Warnings objects have, at a minimum, a `code` and a `message` property, allowing you to control how different kinds of warnings are handled. Other properties are added depending on the type of warning. See [`utils/error.ts`](https://github.com/rollup/rollup/blob/master/src/utils/error.ts) for a complete list of errors and warnings together with their codes and properties.
```js
// rollup.config.js
Expand All @@ -381,7 +381,9 @@ export default {
if (warning.code === 'UNUSED_EXTERNAL_IMPORT') return;

// throw on others
if (warning.code === 'NON_EXISTENT_EXPORT') throw new Error(warning.message);
// Using Object.assign over new Error(warning.message) will make the CLI
// print additional information such as warning location and help url.
if (warning.code === 'MISSING_EXPORT') throw Object.assign(new Error(), warning);

// Use default for everything else
warn(warning);
Expand Down
2 changes: 1 addition & 1 deletion rollup.config.ts
Expand Up @@ -47,7 +47,7 @@ const onwarn: WarningHandlerWithDefault = warning => {
'Building Rollup produced warnings that need to be resolved. ' +
'Please keep in mind that the browser build may never have external dependencies!'
);
throw new Error(warning.message);
throw Object.assign(new Error(), warning);
};

const moduleAliases = {
Expand Down
16 changes: 4 additions & 12 deletions src/Chunk.ts
Expand Up @@ -32,6 +32,8 @@ import { createAddons } from './utils/addons';
import { deconflictChunk, type DependenciesToBeDeconflicted } from './utils/deconflictChunk';
import {
errCyclicCrossChunkReexport,
errEmptyChunk,
errMissingGlobalName,
error,
errUnexpectedNamedImport,
errUnexpectedNamespaceReexport
Expand Down Expand Up @@ -138,12 +140,7 @@ function getGlobalName(
}

if (hasExports) {
warn({
code: 'MISSING_GLOBAL_NAME',
guess: chunk.variableName,
message: `No name was provided for external module '${chunk.id}' in output.globals – guessing '${chunk.variableName}'`,
source: chunk.id
});
warn(errMissingGlobalName(chunk.id, chunk.variableName));
return chunk.variableName;
}
}
Expand Down Expand Up @@ -1148,12 +1145,7 @@ export default class Chunk {
const renderedSource = compact ? magicString : magicString.trim();

if (isEmpty && this.getExportNames().length === 0 && dependencies.size === 0) {
const chunkName = this.getChunkName();
onwarn({
chunkName,
code: 'EMPTY_BUNDLE',
message: `Generated an empty chunk: "${chunkName}"`
});
onwarn(errEmptyChunk(this.getChunkName()));
}
return { accessedGlobals, indent, magicString, renderedSource, usedModules, usesTopLevelAwait };
}
Expand Down
16 changes: 2 additions & 14 deletions src/ExternalModule.ts
@@ -1,10 +1,8 @@
import ExternalVariable from './ast/variables/ExternalVariable';
import type { CustomPluginOptions, ModuleInfo, NormalizedInputOptions } from './rollup/types';
import { EMPTY_ARRAY } from './utils/blank';
import { warnDeprecation } from './utils/error';
import { errUnusedExternalImports, warnDeprecation } from './utils/error';
import { makeLegal } from './utils/identifierHelpers';
import { printQuotedStringList } from './utils/printStringList';
import relativeId from './utils/relativeId';

export default class ExternalModule {
readonly dynamicImporters: string[] = [];
Expand Down Expand Up @@ -105,16 +103,6 @@ export default class ExternalModule {
}
}
const importersArray = [...importersSet];
this.options.onwarn({
code: 'UNUSED_EXTERNAL_IMPORT',
message: `${printQuotedStringList(unused, ['is', 'are'])} imported from external module "${
this.id
}" but never used in ${printQuotedStringList(
importersArray.map(importer => relativeId(importer))
)}.`,
names: unused,
source: this.id,
sources: importersArray
});
this.options.onwarn(errUnusedExternalImports(this.id, unused, importersArray));
}
}

0 comments on commit 7a78648

Please sign in to comment.