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

implement validate output option and --validate CLI option #3952

Merged
merged 6 commits into from Feb 12, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions cli/help.md
Expand Up @@ -72,6 +72,7 @@ Basic options:
--watch.skipWrite Do not write files to disk when watching
--watch.exclude <files> Exclude files from being watched
--watch.include <files> Limit watching to specified files
--validate Validate output

Examples:

Expand Down
2 changes: 2 additions & 0 deletions docs/01-command-line-reference.md
Expand Up @@ -83,6 +83,7 @@ export default { // can be an array (for multiple inputs)
sourcemapExcludeSources,
sourcemapFile,
sourcemapPathTransform,
validate,

// danger zone
amd,
Expand Down Expand Up @@ -330,6 +331,7 @@ Many options have command line equivalents. In those cases, any arguments passed
--watch.skipWrite Do not write files to disk when watching
--watch.exclude <files> Exclude files from being watched
--watch.include <files> Limit watching to specified files
--validate Validate output
```

The flags listed below are only available via the command line interface. All other flags correspond to and override their config file equivalents, see the [big list of options](guide/en/#big-list-of-options) for details.
Expand Down
1 change: 1 addition & 0 deletions docs/02-javascript-api.md
Expand Up @@ -148,6 +148,7 @@ const outputOptions = {
sourcemapExcludeSources,
sourcemapFile,
sourcemapPathTransform,
validate,

// danger zone
amd,
Expand Down
7 changes: 7 additions & 0 deletions docs/999-big-list-of-options.md
Expand Up @@ -924,6 +924,13 @@ export default ({
});
```

#### output.validate
Type: `boolean`<br>
CLI: `--validate`/`--no-validate`<br>
Default: `false`

Re-parses each generated chunk to detect if the generated code is valid JavaScript. This can be useful to debug output generated by plugins that use the [`renderChunk`](guide/en/#renderchunk) hook to transform code.

#### preserveEntrySignatures
Type: `"strict" | "allow-extension" | "exports-only" | false`<br>
CLI: `--preserveEntrySignatures <strict|allow-extension>`/`--no-preserveEntrySignatures`<br>
Expand Down
17 changes: 16 additions & 1 deletion src/Bundle.ts
Expand Up @@ -14,7 +14,12 @@ import {
import { Addons, createAddons } from './utils/addons';
import { getChunkAssignments } from './utils/chunkAssignment';
import commondir from './utils/commondir';
import { errCannotAssignModuleToChunk, error, warnDeprecation } from './utils/error';
import {
errCannotAssignModuleToChunk,
errChunkInvalid,
error,
warnDeprecation
} from './utils/error';
import { sortByExecutionOrder } from './utils/executionOrder';
import { FILE_PLACEHOLDER } from './utils/FileEmitter';
import { basename, isAbsolute } from './utils/path';
Expand Down Expand Up @@ -174,6 +179,16 @@ export default class Bundle {
);
file.type = 'asset';
}
if (this.outputOptions.validate && typeof file.code == 'string') {
try {
this.graph.contextParse(file.code, {
allowHashBang: true,
ecmaVersion: 'latest'
});
} catch (exception) {
return error(errChunkInvalid(file, exception));
}
}
}
this.pluginDriver.finaliseAssets();
}
Expand Down
2 changes: 2 additions & 0 deletions src/rollup/types.d.ts
Expand Up @@ -653,6 +653,7 @@ export interface OutputOptions {
sourcemapPathTransform?: SourcemapPathTransformOption;
strict?: boolean;
systemNullSetters?: boolean;
validate?: boolean;
}

export interface NormalizedOutputOptions {
Expand Down Expand Up @@ -696,6 +697,7 @@ export interface NormalizedOutputOptions {
sourcemapPathTransform: SourcemapPathTransformOption | undefined;
strict: boolean;
systemNullSetters: boolean;
validate: boolean;
}

export type WarningHandlerWithDefault = (
Expand Down
13 changes: 13 additions & 0 deletions src/utils/error.ts
Expand Up @@ -45,6 +45,7 @@ export const enum Errors {
BAD_LOADER = 'BAD_LOADER',
CANNOT_EMIT_FROM_OPTIONS_HOOK = 'CANNOT_EMIT_FROM_OPTIONS_HOOK',
CHUNK_NOT_GENERATED = 'CHUNK_NOT_GENERATED',
CHUNK_INVALID = 'CHUNK_INVALID',
CIRCULAR_REEXPORT = 'CIRCULAR_REEXPORT',
CYCLIC_CROSS_CHUNK_REEXPORT = 'CYCLIC_CROSS_CHUNK_REEXPORT',
DEPRECATED_FEATURE = 'DEPRECATED_FEATURE',
Expand Down Expand Up @@ -93,6 +94,18 @@ export function errChunkNotGeneratedForFileName(name: string) {
};
}

export function errChunkInvalid(
{ fileName, code }: { code: string; fileName: string },
exception: { loc: { column: number; line: number }; message: string }
) {
const errorProps = {
code: Errors.CHUNK_INVALID,
message: `Chunk "${fileName}" is not valid JavaScript: ${exception.message}.`
};
augmentCodeLocation(errorProps, exception.loc, code, fileName);
return errorProps;
}

export function errCircularReexport(exportName: string, importedModule: string) {
return {
code: Errors.CIRCULAR_REEXPORT,
Expand Down
3 changes: 2 additions & 1 deletion src/utils/options/mergeOptions.ts
Expand Up @@ -227,7 +227,8 @@ function mergeOutputOptions(
sourcemapFile: getOption('sourcemapFile'),
sourcemapPathTransform: getOption('sourcemapPathTransform'),
strict: getOption('strict'),
systemNullSetters: getOption('systemNullSetters')
systemNullSetters: getOption('systemNullSetters'),
validate: getOption('validate')
};

warnUnknownOptions(config, Object.keys(outputOptions), 'output options', warn);
Expand Down
3 changes: 2 additions & 1 deletion src/utils/options/normalizeOutputOptions.ts
Expand Up @@ -73,7 +73,8 @@ export function normalizeOutputOptions(
| SourcemapPathTransformOption
| undefined,
strict: (config.strict as boolean | undefined) ?? true,
systemNullSetters: (config.systemNullSetters as boolean | undefined) || false
systemNullSetters: (config.systemNullSetters as boolean | undefined) || false,
validate: (config.validate as boolean | undefined) || false
};

warnUnknownOptions(config, Object.keys(outputOptions), 'output options', inputOptions.onwarn);
Expand Down
3 changes: 2 additions & 1 deletion test/chunking-form/index.js
Expand Up @@ -44,7 +44,8 @@ runTestSuiteWithSamples('chunking form', path.resolve(__dirname, 'samples'), (di
dir: `${dir}/_actual/${format}`,
exports: 'auto',
format,
chunkFileNames: 'generated-[name].js'
chunkFileNames: 'generated-[name].js',
validate: true
},
(config.options || {}).output || {}
),
Expand Down
18 changes: 18 additions & 0 deletions test/cli/samples/validate/_config.js
@@ -0,0 +1,18 @@
const { assertIncludes } = require('../../../utils.js');

module.exports = {
description: 'use CLI --validate to test whether output is well formed',
skipIfWindows: true,
command: `rollup main.js --silent --outro 'console.log("end"); /*' -o _actual/out.js --validate`,
error: () => true,
stderr: stderr =>
assertIncludes(
stderr,
`[!] Error: Chunk "out.js" is not valid JavaScript: Unterminated comment (3:20).
out.js (3:20)
1: console.log(2 );
2:
3: console.log("end"); /*
^`
)
};
1 change: 1 addition & 0 deletions test/cli/samples/validate/main.js
@@ -0,0 +1 @@
console.log(1 ? 2 : 3);
3 changes: 2 additions & 1 deletion test/form/index.js
Expand Up @@ -47,7 +47,8 @@ runTestSuiteWithSamples('form', path.resolve(__dirname, 'samples'), (dir, config
{
exports: 'auto',
file: inputFile,
format: defaultFormat
format: defaultFormat,
// validate: true // add when systemjs bugs fixed
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commented out to pass tests.
Enable validation here to see the issues related to:

test/form/samples/system-export-compact/_expected.js
test/form/samples/system-multiple-export-bindings/_expected.js
test/form/samples/no-external-live-bindings-compact/_expected/system.js

},
(config.options || {}).output || {}
),
Expand Down
3 changes: 2 additions & 1 deletion test/function/samples/output-options-hook/_config.js
Expand Up @@ -45,7 +45,8 @@ module.exports = {
sourcemap: false,
sourcemapExcludeSources: false,
strict: true,
systemNullSetters: false
systemNullSetters: false,
validate: false
});
assert.strictEqual(options.banner(), 'exports.bar = 43;');
assert.ok(/^\d+\.\d+\.\d+/.test(this.meta.rollupVersion));
Expand Down
23 changes: 23 additions & 0 deletions test/function/samples/validate-output/_config.js
@@ -0,0 +1,23 @@
module.exports = {
description: 'handles validate failure',
options: {
output: {
outro: '/*',
validate: true
}
},
generateError: {
code: 'CHUNK_INVALID',
message: 'Chunk "main.js" is not valid JavaScript: Unterminated comment (5:0).',
frame: `
3: throw new Error('Not executed');
4:
5: /*
^`,
loc: {
column: 0,
file: 'main.js',
line: 5
}
}
};
1 change: 1 addition & 0 deletions test/function/samples/validate-output/main.js
@@ -0,0 +1 @@
throw new Error('Not executed');
2 changes: 1 addition & 1 deletion test/misc/misc.js
@@ -1,6 +1,6 @@
const assert = require('assert');
const rollup = require('../../dist/rollup');
const { loader } = require('../utils.js');
const { assertIncludes, loader } = require('../utils.js');

describe('misc', () => {
it('avoids modification of options or their properties', () => {
Expand Down
4 changes: 2 additions & 2 deletions test/misc/optionList.js

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

2 changes: 1 addition & 1 deletion test/test.js
@@ -1,7 +1,7 @@
require('source-map-support').install();

describe('rollup', function () {
this.timeout(15000);
this.timeout(30000);
require('./misc/index.js');
require('./function/index.js');
require('./form/index.js');
Expand Down