Extract option normalization into independant file #125
Conversation
baer
force-pushed
the
feature/extract-option-validation
branch
from
d41370c to
0c9d4f0
Compare
| @@ -51,7 +34,7 @@ export const isPluginRequired = (supportedEnvironments, plugin) => { | |||
| const lowestTargetedVersion = supportedEnvironments[environment]; | |||
|
|
|||
| if (typeof lowestTargetedVersion === "string") { | |||
| throw new Error(`Target version must be a number, | |||
| throw new Error(`Target version must be a number, | |||
| '${lowestTargetedVersion}' was given for '${environment}'`); | |||
There was a problem hiding this comment.
invariant(
typeof lowestTargetedVersion === "number",
`Target version must be a number, '${lowestTargetedVersion}' was given for '${environment}'`
);
There was a problem hiding this comment.
Yes! I tried not to touch index.js anywhere that wasn't related to extracting option parsing but if this PR goes through I'd love to follow up with one to improve all logging and error messages like this.
baer
force-pushed
the
feature/extract-option-validation
branch
from
0c9d4f0 to
1dc7238
Compare
|
NOTE: I now see that #124 added a yarnfile - this PR does not update the yarnfile.lock. I am working to get yarn running properly on this machine - do not merge on my behalf until I make that fix. |
baer
force-pushed
the
feature/extract-option-validation
branch
from
1dc7238 to
4cd5c28
Compare
|
Now that #145 is in, this should be okay to merge. |
|
sure 👍 |
| if (opts.whitelist && !hasBeenWarned) { | ||
| console.warn( | ||
| `Deprecation Warning: The "whitelist" option has been deprecated in favor of "include" to | ||
| matchthe newly added "exclude" option (instead of "blacklist").` |
There was a problem hiding this comment.
Nit: matchthe -> match the
|
|
||
| invariant( | ||
| unknownOpts.length === 0, | ||
| `Invalid Option: The ${type} of '${unknownOpts}' is not a valid plugins/built-in. Please check |
There was a problem hiding this comment.
Reads sort of strange:
The
excludeof${unknownOpts}is not a valid plugin/built-ins
Thoughts on just tweaking the original?
Invalid Option: The plugins/built-ins '${unknownOpts}' passed to the '${type}' option are not valid.
baer
force-pushed
the
feature/extract-option-validation
branch
from
4cd5c28 to
b8c768f
Compare
This PR moves all preset option validation logic into a separate file. 95% of this PR is simply moving code from the
index.js=>normalize-options.jswith the exception of the following:Invalid Option: <problem>orDeprecation Warning: <problem>invariantlibrary was pulled in to help make the code a bit more readable.normalizeOptionsdoes not mutate or change the input object. Part of what set me down the path of this refactor was unexpectedly finding thatvalidateIncludeOptionandvalidateExcludeOptionnot only altered the input but changed the shape.lodash/intersectionwas pulled into the plugin bundle for clarity.Thanks for all the great work on this repo! If you find this change too invasive, I'm happy to try to scale back in places. The impetus for all of this was finding that validation was actually mutating which could be fixed in other ways. Thanks for doing all ya'll do here!