New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: ensure array types #4266
base: master
Are you sure you want to change the base?
fix: ensure array types #4266
Conversation
ad1daa0
to
f4af4ca
Compare
Codecov Report
@@ Coverage Diff @@
## master #4266 +/- ##
==========================================
- Coverage 98.39% 98.39% -0.01%
==========================================
Files 204 204
Lines 7312 7310 -2
Branches 2084 2083 -1
==========================================
- Hits 7195 7193 -2
Misses 58 58
Partials 59 59
Continue to review full report at Codecov.
|
@@ -53,7 +54,7 @@ export function normalizeInputOptions(config: InputOptions): { | |||
moduleContext: getModuleContext(config, context), | |||
onwarn, | |||
perf: config.perf || false, | |||
plugins: ensureArray(config.plugins), | |||
plugins: ensureArray(config.plugins) as Plugin[], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that an improvement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that an improvement?
🤣
depends on the point of view. currently, maybe not so much. when I cruise thru the code I see a lot of type assertions and non-null assertions (!), where I feel like I want to fix those. the problem is that no matter which route you go you always find yourself really fast in a rabbit hole 😉 because the intensions are not clear. so usually I revert and let it be.
the current ensure array typing is incorrect in the sense that the generic < T > already covers false | undefined | null
. I believe it's just there to make the type checker happy, but the root problem is elsewhere.
that's why I left this PR as a draft, I'll have a couple questions when I get back to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure 😉, I really like that you look through the code search for ways to make it better!
e512052
to
5abf7a1
Compare
eca6a6c
to
71d20c9
Compare
cbd94e6
to
f1538ce
Compare
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description