(types/refactor): be more specific than 'any' in build code #401
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
swyxio
reviewed
Dec 28, 2019
swyxio
approved these changes
Dec 28, 2019
- add types for all options and use those types throughout the
build/watch code
- fix some incorrect types in a few places that became more obvious
or errors once types were added
- fix default target from 'web' (not an option) to 'browser'
- extract createBuildConfigs into a separate file as it's fairly long
and has some relatively complex code
- and refactor it a bit to make it a bit easier to read as well as
easier to type (and type cast)
agilgur5
force-pushed
the
more-build-types
branch
from
28a267b
to
98249b4
Compare
jaredpalmer
approved these changes
Dec 29, 2019
|
Resolve conflicts and then lgtm |
|
will merge when the tests pass |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
add types for all options and use those types throughout the
build/watch code
fix some incorrect types in a few places that became more obvious
or errors once types were added
fix default target from 'web' (not an option) to 'browser'
extract createBuildConfigs into a separate file as it's fairly long
and has some relatively complex code
easier to type (and type cast)
Follow-up to #371 , which was extracted out of this before I finished. I originally started this after #367 as the added types would give a lot of extra confidence around options, which were previously
anyeverywhere. This is also going to merge conflict quite hard against #367 .Review Notes
There are two type casts I couldn't seem to get around here (guess it hits the limits of TypeScript's inference?), but this is still much better than all the
anys, and now new options must be typed and then used properly.Future Work
createAllFormats(and possibly some of the code around it) is one of those things that could probably be split into a separate rollup plugin.