feat: Migrate bundle analysis services to Query V5 #3430
Conversation
Bundle ReportChanges will increase total bundle size by 24.36kB (0.14%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
Bundle ReportChanges will increase total bundle size by 24.36kB (0.14%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #3430 +/- ##
==========================================
- Coverage 99.09% 99.08% -0.01%
==========================================
Files 804 804
Lines 14187 14184 -3
Branches 4017 4009 -8
==========================================
- Hits 14058 14054 -4
- Misses 120 121 +1
Partials 9 9
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3430 +/- ##
==========================================
- Coverage 99.09% 99.08% -0.01%
==========================================
Files 804 804
Lines 14187 14184 -3
Branches 4024 4016 -8
==========================================
- Hits 14058 14054 -4
- Misses 120 121 +1
Partials 9 9
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3430 +/- ##
==========================================
- Coverage 99.09% 99.08% -0.01%
==========================================
Files 804 804
Lines 14187 14184 -3
Branches 4024 4016 -8
==========================================
- Hits 14058 14054 -4
- Misses 120 121 +1
Partials 9 9
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3430 +/- ##
==========================================
- Coverage 99.09% 99.08% -0.01%
==========================================
Files 804 804
Lines 14187 14184 -3
Branches 4017 4009 -8
==========================================
- Hits 14058 14054 -4
- Misses 120 121 +1
Partials 9 9
Continue to review full report in Codecov by Sentry.
|
✅ Deploy preview for gazebo ready!Previews expire after 1 month automatically.
|
nicholas-codecov
force-pushed
the
gh-eng-2128-migrate-bundle-analysis-services-to-V5
branch
from
a3af02a
to
3e11000
Compare
nicholas-codecov
changed the title
| @@ -161,7 +161,7 @@ const createColumns = (totalBundleSize: number | null) => [ | |||
| />{' '} | |||
| <ChangeOverTime | |||
| change={value?.change?.size.uncompress} | |||
| hasMeasurements={value.measurements.length > 0 ?? false} | |||
| hasMeasurements={value.measurements.length > 0} | |||
There was a problem hiding this comment.
This was something that Vite kept bringing up as an optimization so finally threw it in.
nicholas-codecov
force-pushed
the
gh-eng-2128-migrate-bundle-analysis-services-to-V5
branch
from
833a3d0
to
d36c7dc
Compare
| provider: string | ||
| owner: string | ||
| repo: string | ||
| branch?: string | ||
| branch: string | null | undefined |
There was a problem hiding this comment.
Why this instead of branch?: string | null?
There was a problem hiding this comment.
This is an attempt to be a bit more explicit with function args and their types. ? represents an optional argument so TS doesn't complain if you never provide it. However, if you type it as above, TS will complain if you don't provide the value:
There was a problem hiding this comment.
Oh so you just want this to always be called with a branch param?
There was a problem hiding this comment.
Yes I would like all the arguments provided, this still makes allowances however if branch is undefined.
| @@ -266,32 +271,34 @@ export const useBundleAssets = ({ | |||
| dateBefore, | |||
| dateAfter, | |||
| filters, | |||
| assetsAfter: pageParam, | |||
| assetsAfter, | |||
There was a problem hiding this comment.
Does this mean that assetsAfter=null doesn't get along well with the query? I'd think it might be worthwhile updating this API side if not. Would rather do that than do the few lines above on every infinite query.
There was a problem hiding this comment.
Sorry, null works fine, and I'm updating things rn to be set to null instead of undefined. The type for pageParam is inferred through the initialPageParam so we have to assign it to a string for the remaining cursors to keep the types happy.
There was a problem hiding this comment.
If null is fine, doesn't the old code work? assetsAfter: pageParam
There was a problem hiding this comment.
I may be misunderstanding though
There was a problem hiding this comment.
Soooo just did some testing, and the API is okay if just an empty string is provided ... so I'll get rid of all of this.
Description
This PR updates the hooks/services in the
services/bundleAnalysisdirectory to Query V5. Some of these updates include moving fully over to thequeryOptionsAPI format that we will be moving towards, I didn't do this for all of these, as the PR would have grown significantly, and can be accomplished in a follow up PR.Another change I made in this PR was the removal of the
index.tsfile from theservices/bundleAnalysisdirectory. Some of the benefits of this are:export * from './...'cmd + clickon the import you're taken to the function declaration not theindex.tsfileTicket: codecov/engineering-team#2128
Notable Changes
services/bundleAnalysisto Query V5queryOptionAPI format