Typecheck much more of the config loading process #5642
Conversation
|
@loganfsmyth, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jamestalmage, @existentialism and @forivall to be potential reviewers. |
Codecov Report
@@ Coverage Diff @@
## 7.0 #5642 +/- ##
==========================================
- Coverage 84.51% 84.36% -0.15%
==========================================
Files 285 285
Lines 9737 9761 +24
Branches 2723 2737 +14
==========================================
+ Hits 8229 8235 +6
- Misses 1003 1010 +7
- Partials 505 516 +11
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This LGTM, nice work @loganfsmyth!
I'm wondering if we could just all=true in flow config to enable type check on all files.
| if (options.env) { | ||
| const envOpts = options.env[envKey]; | ||
| delete options.env; | ||
| const envKey = getEnv(); |
There was a problem hiding this comment.
nit: the comment above is not useful, do you mind removing it?
|
|
||
| if (rawOpts.env != null && (typeof rawOpts.env !== "object" || Array.isArray(rawOpts.env))) { | ||
| throw new Error(".env block must be an object, null, or undefined"); |
There was a problem hiding this comment.
I think we should use a helper function for this kind of error messages.
We can also clarify a bit the message:
The `env` key in your Babel configuration must be ..., ... given
And maybe link to the website?
There was a problem hiding this comment.
Yeah possibly. I've got more work planned to give more information about the errors, but that's a few PRs from now.
| @@ -12,11 +14,11 @@ import clone from "lodash/clone"; | |||
| import { loadPlugin, loadPreset, loadParser, loadGenerator } from "./loading/files"; | |||
|
|
|||
| type MergeOptions = { | |||
| type: "arguments"|"options"|"preset", | |||
| options?: Object, | |||
| +type: "arguments"|"options"|"preset", | |||
There was a problem hiding this comment.
Just for my information, what is the + sign for?
There was a problem hiding this comment.
This is property covariance. In this case it essentially allows two types that are mostly-compatible to play nicely together. Take this case:
type ConfigItem = {
type: "options"|"arguments",
};
type MergeOptions = {
type: "arguments"|"options"|"preset",
};
var item: ConfigItem = {
type: "options",
};
var merge: MergeOptions = item;
This will fail, because merge.type = "preset"; would cause item to no longer match its type properly, since ConfigItem objects cant be have type preset.
By adding the +, you are basically saying "don't let me reassign this", so
var item: ConfigItem = {
type: "options",
};
var merge: MergeOptions = item;
will work, but it will throw an error if you try to reassign merge.type.
| const options = normalizeOptions(config); | ||
|
|
||
| if (config.options.plugins != null && !Array.isArray(config.options.plugins)) { | ||
| throw new Error(".plugins should be an array, null, or undefined"); |
There was a problem hiding this comment.
Same comment above for this and below
hzoo
added
the
PR: Internal 🏠
* 'master' of github.com:hulkish/babel: (190 commits) Fix incorrect property ordering with obj rest spread on nested (babel#5685) Fix PathHoister hoisting before a same-scope variable declaration. Updated transform-react-display-name for createReactClass addon (babel#5554) Fix PathHoister error attaching after export declarations. add .mjs to list of well known extensions Remove babel-helper-builder-conditional-assignment-operator-visitor, unused in babel [skip ci] (babel#5676) use find-cache-dir for babel-register cache (babel#5669) Fix operator processing in object super. -> parsedAst string -> sourceCode, ast -> generatedCode back to babylon Switch to pirates for babel-register. (babel#3670) [skip ci] babylon -> babel, ast -> parsedAst [readme] change code -> string Add support for object type spread (babel#5525) Fix object destructuring in param arrays (babel#5650) Remove merge helper and add more type declarations. (babel#5649) Typecheck much more of the config loading process (babel#5642) update to alpha.9 (babel#5639) v7.0.0-alpha.9 ...
* '7.0' of https://github.com/babel/babel: (190 commits) Fix incorrect property ordering with obj rest spread on nested (babel#5685) Fix PathHoister hoisting before a same-scope variable declaration. Updated transform-react-display-name for createReactClass addon (babel#5554) Fix PathHoister error attaching after export declarations. add .mjs to list of well known extensions Remove babel-helper-builder-conditional-assignment-operator-visitor, unused in babel [skip ci] (babel#5676) use find-cache-dir for babel-register cache (babel#5669) Fix operator processing in object super. -> parsedAst string -> sourceCode, ast -> generatedCode back to babylon Switch to pirates for babel-register. (babel#3670) [skip ci] babylon -> babel, ast -> parsedAst [readme] change code -> string Add support for object type spread (babel#5525) Fix object destructuring in param arrays (babel#5650) Remove merge helper and add more type declarations. (babel#5649) Typecheck much more of the config loading process (babel#5642) update to alpha.9 (babel#5639) v7.0.0-alpha.9 ...
lock
bot
added
the
outdated
Just adding a bunch of Flowtype annotations to the config folder files. With this, everything in
babel-core/src/confighas an@flowdeclaration.There are still a few places typed generic
Object, but this gets rid of most of them except for the actual resultoptionsobject, which has not changed and is only partially validated.