refactor: some code and type fixes #4413
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4413 +/- ##
==========================================
- Coverage 98.75% 98.75% -0.01%
==========================================
Files 204 204
Lines 7322 7299 -23
Branches 2079 2078 -1
==========================================
- Hits 7231 7208 -23
Misses 33 33
Partials 58 58
Continue to review full report at Codecov.
|
dnalborczyk
force-pushed
the
some-fixes
branch
from
baa259b
to
bdcbe6d
Compare
dnalborczyk
force-pushed
the
some-fixes
branch
from
c4614cd
to
c3dbca6
Compare
dnalborczyk
force-pushed
the
some-fixes
branch
from
78ce7df
to
9cdfc9b
Compare
dnalborczyk
force-pushed
the
some-fixes
branch
2 times, most recently
from
0454980
to
4be1a20
Compare
dnalborczyk
force-pushed
the
some-fixes
branch
from
4be1a20
to
33446da
Compare
dnalborczyk
force-pushed
the
some-fixes
branch
from
93fde1d
to
b4df2b4
Compare
src/Chunk.ts
Outdated
| }) | ||
| module.chunkNames | ||
| .filter(({ isUserDefined }) => isUserDefined) | ||
| .map(({ name }) => ({ name })) |
There was a problem hiding this comment.
This isn't the same as it won't do name string deduping in the set.
There was a problem hiding this comment.
great catch, thank you! too bad this wasn't caught by a test 😞 I reverted and added a comment.
There was a problem hiding this comment.
Actually I think it does not change anything at the moment, which is why there is no test. The only situation where you have entries here is if the user used an object as entry point. In that case, the name properties are the keys of that object, and object keys deduplicate themselves. It does not hurt either, though.
| specifier, | ||
| module.id | ||
| )); | ||
| return (module.resolvedIds[specifier] ??= this.handleResolveId( |
There was a problem hiding this comment.
Are we using ?? in the codebase now?
There was a problem hiding this comment.
yeah, nullish coalescing is used throughout the codebase, logical nullish assignment a little bit.
down-leveling only adds a tiny amount of additional code which results in almost the same amount without the change. if v3 were about to support node.js v14+ nullish coalescing would also run native as well. short circuiting should also make logical nullish assignment faster.
the correct syntax mapping would have been ||= rather, but I thought since ResolveId is an object, ??= would fit better to show the intend of only assigning when null or undefined.
…unction" This reverts commit 055f4a0.
dnalborczyk
force-pushed
the
some-fixes
branch
from
58e8230
to
da6dee6
Compare
dnalborczyk
force-pushed
the
some-fixes
branch
2 times, most recently
from
a9ae872
to
847fc1a
Compare
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description