Skip to content

Commit

Permalink
polish: throw human-friendly error when item-option pair is in… (#10969)
Browse files Browse the repository at this point in the history
* polish: throw human-friendly error when item-option pair is incorrectly unwrapped

* add testcase for plugin

* fix: exclude false positive

* fix: validate should support plugin optionsSourceKind

* Revert "fix: validate should support plugin optionsSourceKind"

* fix: validate plugin object in assertNoUnwrappedItemOptionPairs

* fix flow error

* update test fixtures

* refactor: move to loadDescriptor catch clause

* chore: throw Error instead of builtin ReferenceError

* fix flow errors

* chore: add more test cases
  • Loading branch information
JLHwung authored and nicolo-ribaudo committed Jan 20, 2020
1 parent 43b23e0 commit fa975bf
Show file tree
Hide file tree
Showing 7 changed files with 164 additions and 14 deletions.
49 changes: 41 additions & 8 deletions packages/babel-core/src/config/full.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ import {
makeWeakCacheSync,
type CacheConfigurator,
} from "./caching";
import { validate, type CallerMetadata } from "./validation/options";
import {
validate,
type CallerMetadata,
checkNoUnwrappedItemOptionPairs,
} from "./validation/options";
import { validatePluginObject } from "./validation/plugins";
import makeAPI from "./helpers/config-api";

Expand Down Expand Up @@ -78,19 +82,48 @@ export default gensync<[any], ResolvedConfig | null>(function* loadFullConfig(
pass: Array<Plugin>,
) {
const plugins = [];
for (const descriptor of config.plugins) {
for (let i = 0; i < config.plugins.length; i++) {
const descriptor = config.plugins[i];
if (descriptor.options !== false) {
plugins.push(yield* loadPluginDescriptor(descriptor, context));
try {
plugins.push(yield* loadPluginDescriptor(descriptor, context));
} catch (e) {
// print special message for `plugins: ["@babel/foo", { foo: "option" }]`
if (i > 0 && e.code === "BABEL_UNKNOWN_PLUGIN_PROPERTY") {
checkNoUnwrappedItemOptionPairs(
config.plugins[i - 1],
descriptor,
"plugin",
i,
e,
);
}
throw e;
}
}
}

const presets = [];
for (const descriptor of config.presets) {
for (let i = 0; i < config.presets.length; i++) {
const descriptor = config.presets[i];
if (descriptor.options !== false) {
presets.push({
preset: yield* loadPresetDescriptor(descriptor, context),
pass: descriptor.ownPass ? [] : pass,
});
try {
presets.push({
preset: yield* loadPresetDescriptor(descriptor, context),
pass: descriptor.ownPass ? [] : pass,
});
} catch (e) {
if (i > 0 && e.code === "BABEL_UNKNOWN_OPTION") {
checkNoUnwrappedItemOptionPairs(
config.presets[i - 1],
descriptor,
"preset",
i,
e,
);
}
throw e;
}
}
}

Expand Down
38 changes: 33 additions & 5 deletions packages/babel-core/src/config/validation/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
type Validator,
type OptionPath,
} from "./option-assertions";
import type { UnloadedDescriptor } from "../config-descriptors";

const ROOT_VALIDATORS: ValidatorSet = {
cwd: (assertString: Validator<$PropertyType<ValidatedOptions, "cwd">>),
Expand Down Expand Up @@ -365,16 +366,20 @@ function throwUnknownError(loc: OptionPath) {
if (removed[key]) {
const { message, version = 5 } = removed[key];

throw new ReferenceError(
throw new Error(
`Using removed Babel ${version} option: ${msg(loc)} - ${message}`,
);
} else {
// eslint-disable-next-line max-len
const unknownOptErr = `Unknown option: ${msg(
loc,
)}. Check out https://babeljs.io/docs/en/babel-core/#options for more information about options.`;
const unknownOptErr = new Error(
`Unknown option: ${msg(
loc,
)}. Check out https://babeljs.io/docs/en/babel-core/#options for more information about options.`,
);
// $FlowIgnore
unknownOptErr.code = "BABEL_UNKNOWN_OPTION";

throw new ReferenceError(unknownOptErr);
throw unknownOptErr;
}
}

Expand Down Expand Up @@ -439,3 +444,26 @@ function assertOverridesList(loc: OptionPath, value: mixed): OverridesList {
}
return (arr: any);
}

export function checkNoUnwrappedItemOptionPairs(
lastItem: UnloadedDescriptor,
thisItem: UnloadedDescriptor,
type: "plugin" | "preset",
index: number,
e: Error,
): void {
if (
lastItem.file &&
lastItem.options === undefined &&
typeof thisItem.value === "object"
) {
e.message +=
`\n- Maybe you meant to use\n` +
`"${type}": [\n ["${lastItem.file.request}", ${JSON.stringify(
thisItem.value,
undefined,
2,
)}]\n]\n` +
`To be a valid ${type}, its name and options should be wrapped in a pair of brackets`;
}
}
9 changes: 8 additions & 1 deletion packages/babel-core/src/config/validation/plugins.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,14 @@ export function validatePluginObject(obj: {}): PluginObject {
};

if (validator) validator(optLoc, obj[key]);
else throw new Error(`.${key} is not a valid Plugin property`);
else {
const invalidPluginPropertyError = new Error(
`.${key} is not a valid Plugin property`,
);
// $FlowIgnore
invalidPluginPropertyError.code = "BABEL_UNKNOWN_PLUGIN_PROPERTY";
throw invalidPluginPropertyError;
}
});

return (obj: any);
Expand Down
34 changes: 34 additions & 0 deletions packages/babel-core/test/__snapshots__/option-manager.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`option-manager config plugin/preset flattening and overriding should throw when an option is following a preset 1`] = `
"[BABEL] unknown: Unknown option: .useSpread. Check out https://babeljs.io/docs/en/babel-core/#options for more information about options.
- Maybe you meant to use
\\"preset\\": [
[\\"./fixtures/option-manager/babel-preset-bar\\", {
\\"useSpread\\": true
}]
]
To be a valid preset, its name and options should be wrapped in a pair of brackets"
`;

exports[`option-manager config plugin/preset flattening and overriding should throw when an option is provided as a plugin 1`] = `
"[BABEL] unknown: .useSpread is not a valid Plugin property
- Maybe you meant to use
\\"plugin\\": [
[\\"./fixtures/option-manager/babel-plugin-foo\\", {
\\"useSpread\\": true
}]
]
To be a valid plugin, its name and options should be wrapped in a pair of brackets"
`;

exports[`option-manager config plugin/preset flattening and overriding should throw when an option is provided as a preset 1`] = `
"[BABEL] unknown: Unknown option: .useBuiltIns. Check out https://babeljs.io/docs/en/babel-core/#options for more information about options.
- Maybe you meant to use
\\"preset\\": [
[\\"./fixtures/option-manager/babel-preset-bar\\", {
\\"useBuiltIns\\": \\"entry\\"
}]
]
To be a valid preset, its name and options should be wrapped in a pair of brackets"
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = () => ({});
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = () => ({});
46 changes: 46 additions & 0 deletions packages/babel-core/test/option-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,52 @@ describe("option-manager", () => {
return { plugin, calls };
}

it("should throw when an option is provided as a preset", () => {
expect(() => {
loadOptions({
presets: [
"./fixtures/option-manager/babel-preset-bar",
{ useBuiltIns: "entry" },
],
});
}).toThrowErrorMatchingSnapshot();
});

it("should throw when an option is provided as a plugin", () => {
expect(() => {
loadOptions({
plugins: [
"./fixtures/option-manager/babel-plugin-foo",
{ useSpread: true },
],
});
}).toThrowErrorMatchingSnapshot();
});

it("should throw when an option is following a preset", () => {
expect(() => {
loadOptions({
presets: [
"./fixtures/option-manager/babel-plugin-foo",
"./fixtures/option-manager/babel-preset-bar",
{ useSpread: true },
],
});
}).toThrowErrorMatchingSnapshot();
});

it("should not throw when a preset string followed by valid preset object", () => {
const { plugin } = makePlugin("my-plugin");
expect(
loadOptions({
presets: [
"@babel/env",
{ plugins: [[plugin, undefined, "my-plugin"]] },
],
}),
).toBeTruthy();
});

it("should throw if a plugin is repeated, with information about the repeated plugin", () => {
const { calls, plugin } = makePlugin("my-plugin");

Expand Down

0 comments on commit fa975bf

Please sign in to comment.