-
-
Notifications
You must be signed in to change notification settings - Fork 93
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
enhance: Improve args type matching for hooks #3020
Conversation
🦋 Changeset detectedLatest commit: d31c53a The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3020 +/- ##
=========================================
Coverage ? 98.49%
=========================================
Files ? 122
Lines ? 2186
Branches ? 439
=========================================
Hits ? 2153
Misses ? 21
Partials ? 12 ☔ View full report in Codecov by Sentry. |
fc13ecb
to
2fd4521
Compare
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.
Benchmark
Benchmark suite | Current: d31c53a | Previous: 4e009d3 | Ratio |
---|---|---|---|
normalizeLong |
448 ops/sec (±2.01% ) |
433 ops/sec (±1.87% ) |
0.97 |
infer All |
9700 ops/sec (±0.39% ) |
9603 ops/sec (±0.54% ) |
0.99 |
denormalizeLong |
344 ops/sec (±0.97% ) |
307 ops/sec (±2.49% ) |
0.89 |
denormalizeLong donotcache |
898 ops/sec (±0.36% ) |
854 ops/sec (±1.16% ) |
0.95 |
denormalizeShort donotcache 500x |
1377 ops/sec (±0.23% ) |
1358 ops/sec (±0.94% ) |
0.99 |
denormalizeShort 500x |
982 ops/sec (±0.29% ) |
980 ops/sec (±0.21% ) |
1.00 |
denormalizeShort 500x withCache |
4916 ops/sec (±0.15% ) |
4904 ops/sec (±0.33% ) |
1.00 |
denormalizeLong with mixin Entity |
298 ops/sec (±0.28% ) |
292 ops/sec (±0.34% ) |
0.98 |
denormalizeLong withCache |
6943 ops/sec (±0.22% ) |
6570 ops/sec (±0.23% ) |
0.95 |
denormalizeLong All withCache |
6757 ops/sec (±0.12% ) |
6772 ops/sec (±0.23% ) |
1.00 |
denormalizeLong Query-sorted withCache |
6557 ops/sec (±0.60% ) |
6580 ops/sec (±0.72% ) |
1.00 |
denormalizeLongAndShort withEntityCacheOnly |
1552 ops/sec (±0.47% ) |
1517 ops/sec (±0.67% ) |
0.98 |
getResponse |
6235 ops/sec (±1.21% ) |
5793 ops/sec (±1.19% ) |
0.93 |
getResponse (null) |
6054545 ops/sec (±0.88% ) |
5833897 ops/sec (±0.89% ) |
0.96 |
getResponse (clear cache) |
292 ops/sec (±0.35% ) |
287 ops/sec (±0.52% ) |
0.98 |
getSmallResponse |
2687 ops/sec (±0.21% ) |
2651 ops/sec (±0.45% ) |
0.99 |
getSmallInferredResponse |
2067 ops/sec (±0.14% ) |
1956 ops/sec (±0.12% ) |
0.95 |
getResponse Query-sorted |
5490 ops/sec (±0.73% ) |
5511 ops/sec (±0.44% ) |
1.00 |
getResponse Collection |
6597 ops/sec (±1.17% ) |
6037 ops/sec (±0.78% ) |
0.92 |
get Collection |
5647 ops/sec (±0.99% ) |
5282 ops/sec (±1.32% ) |
0.94 |
setLong |
453 ops/sec (±2.04% ) |
427 ops/sec (±2.19% ) |
0.94 |
setLongWithMerge |
189 ops/sec (±1.31% ) |
179 ops/sec (±1.74% ) |
0.95 |
setLongWithSimpleMerge |
197 ops/sec (±1.24% ) |
187 ops/sec (±1.64% ) |
0.95 |
setSmallResponse 500x |
868 ops/sec (±1.60% ) |
833 ops/sec (±1.52% ) |
0.96 |
This comment was automatically generated by workflow using github-action-benchmark.
96b12f0
to
d821c09
Compare
d821c09
to
d31c53a
Compare
Motivation
Precisely match args:
In some cases matching args would simply throw off the return value. In the case above it should match exactly, but it is thrown of by the string literals. This results in the return condition finding null in the union.
Solution
Previously we used a Generic for Args rather than a function union to correctly handle args with union types (e.g.,
id ? { id } : null
).However, we now realize that we don't really care about handling the strict null arg case's return type. Therefore, we don't need to match the strict null case. So instead one of our functions can be
Parameters<E> | [null]
, which handles the union type above just fine.Since we are no longer using Generics, we throw in NoInfer<> to keep matching accurate.
To maintain back compatibility with TS < 5.4, we add our own
NI<>
utility that simply is the identity type for versions less than 5.4.Legacy type utility
We add
build-legacy-types.sh
which more easily handles generating legacy typescript versions to support many different versions.