feat(errors): validate preset when filename is absent#10181
feat(errors): validate preset when filename is absent#10181nicolo-ribaudo merged 4 commits intobabel:masterfrom
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/11468/ |
| if (options.test || options.include || options.exclude) { | ||
| throw new Error( | ||
| `Preset ${descriptor.name || | ||
| ""} requires filename, but it was not passed.`.replace(/\s{2}/, " "), |
There was a problem hiding this comment.
Might be nice to link directly to the option:
Preset ${name} requires a filename be set. See https://babeljs.io/docs/en/options#filename.
Or even:
Preset ${name} requires a filename be set. For example, if you are calling Babel directly:
babel.transform(code, { filename: 'file.js', presets: [${name || '/* your preset */'}] });See https://babeljs.io/docs/en/options#filename for more information.
(And, as you mentioned, improve this later with user- and preset-specific messages)
There was a problem hiding this comment.
That delivers way better dev UX. Addressed in cf0f27a.
|
Should we also update the language on the options page (https://babeljs.io/docs/en/options#filename)? Maybe simply changing:
to:
Or, since we get the question a bunch, call out preset-typescript's behavior directly here? (We can continue discussion on the website repo after this lands) |
| ): void => { | ||
| if (!context.filename) { | ||
| const { options } = preset; | ||
| if (options.test || options.include || options.exclude) { |
There was a problem hiding this comment.
We should also check if the overrides option contains a block which has one of these three options (like in the TS preset).
ffc6a72 to
cf0f27a
Compare
|
(Note: I labeled this as "New feature" but I I think that this PR can be merged in a patch version) |
| : "/* your preset */"; | ||
| throw new Error( | ||
| [ | ||
| `Preset ${formattedPresetName} requires a filename be set.`, |
There was a problem hiding this comment.
I read again and feel like
Preset ${formattedPresetName} requires a filename to be set when babel is called directly,
`\`\`\``,
`babel.transform(code, { filename: 'file.ts', presets: [${formattedPresetName}] });`,
`\`\`\``
And we may can change file.js to file.ts since this is the popular path.
There was a problem hiding this comment.
And we may can change file.js to file.ts since this is the popular path
Good idea
cf0f27a to
4597db5
Compare
Closes #10154
Validate preset before
buildPresetChain, when it is the last chance we can get theUnloadedDescriptorand include preset name in error message. We may add more business logic insidevalidatePresetto print preset-specific error message.I have also considered the following alternative designs:
Pass down descriptor name to
matchPattern.Pro: The missing filename errors are still thrown in
matchPatternCons: Introduce unnecessary signature complexity as this is not our primary execution path
Add descriptor name to
ConfigContextPro: The missing filename errors are still thrown in
matchPatternCons: ConfigContext has to be mutated or cloned when running
buildPresetChain. TheConfigContextis widely used across config chain building but descriptor name is only used to print this error.tryCatch
buildPresetChainPros: Intuitive and fewer execution path
Cons: Any error from
buildPresetChainhas to be matched once before they are re-thrown.