chore: Convert to TS - services selfHosted, access, pull, nav #3439
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #3439 +/- ##
==========================================
- Coverage 99.10% 99.09% -0.02%
==========================================
Files 804 804
Lines 14125 14187 +62
Branches 3961 4024 +63
==========================================
+ Hits 13998 14058 +60
- Misses 118 120 +2
Partials 9 9
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3439 +/- ##
==========================================
- Coverage 99.10% 99.09% -0.02%
==========================================
Files 804 804
Lines 14125 14187 +62
Branches 3961 4017 +56
==========================================
+ Hits 13998 14058 +60
- Misses 118 120 +2
Partials 9 9
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3439 +/- ##
==========================================
- Coverage 99.10% 99.09% -0.02%
==========================================
Files 804 804
Lines 14125 14187 +62
Branches 3961 4024 +63
==========================================
+ Hits 13998 14058 +60
- Misses 118 120 +2
Partials 9 9
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #3439 +/- ##
==========================================
- Coverage 99.10% 99.09% -0.02%
==========================================
Files 804 804
Lines 14125 14187 +62
Branches 3961 4024 +63
==========================================
+ Hits 13998 14058 +60
- Misses 118 120 +2
Partials 9 9
Continue to review full report in Codecov by Sentry.
|
Bundle ReportChanges will increase total bundle size by 1.1kB (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
Bundle ReportChanges will increase total bundle size by 1.1kB (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
✅ Deploy preview for gazebo ready!Previews expire after 1 month automatically.
|
|
|
||
| const { result } = renderHook( | ||
| () => useGenerateUserToken({ provider }), | ||
| { wrapper } | ||
| ) | ||
|
|
||
| result.current.mutate({ sessionid: 1 }) | ||
| result.current.mutate({ name: '1' }) |
There was a problem hiding this comment.
test was wrong before
| @@ -2,7 +2,7 @@ import omitBy from 'lodash/omitBy' | |||
| import qs from 'qs' | |||
| import { useHistory, useLocation } from 'react-router-dom' | |||
|
|
|||
| export function useLocationParams(defaultParams = {}) { | |||
| export function useLocationParams(defaultParams: Record<string, unknown> = {}) { | |||
There was a problem hiding this comment.
I wonder if we can use some TS generics here so that the default params that we pass in are typed to the return value of params
There was a problem hiding this comment.
And i wonder if we can utilize these same generics to the update functions etc. so that when we call them we know the keys, and their value types.
There was a problem hiding this comment.
I think I had it most of the way there in this file but the changes it touched started to explode to 10+ other files and fixes needed. Spun it into this ticket and I can take another stab later - codecov/engineering-team#2843
There was a problem hiding this comment.
If it's gonna be a significant investment, we may want to skip it, and wait until we move routers to save time ... and potentially lots of hair pulling.
| return Promise.reject({ | ||
| status: 404, | ||
| data: {}, | ||
| dev: 'useSelfHostedSeatsConfig - 404 schema parsing failed', | ||
| } satisfies NetworkErrorObject) |
There was a problem hiding this comment.
Can we use the new rejectNetworkError helper function here 👀
| } satisfies NetworkErrorObject) | ||
| } | ||
|
|
||
| return parsedRes ?? null |
There was a problem hiding this comment.
I think we need to use the parsedRes.data field here so we only get the data sent from the API over the that plus the zod info
There was a problem hiding this comment.
select below it was pulling out data. I adjusted these 2 to fit the pattern in the rest of our app
| return Promise.reject({ | ||
| status: 404, | ||
| data: {}, | ||
| dev: 'useSelfHostedHasAdmins - 404 schema parsing failed', | ||
| } satisfies NetworkErrorObject) | ||
| } | ||
|
|
||
| return parsedRes |
There was a problem hiding this comment.
Another instance of select manipulating the data. I changed it to look like other places in our app.
updated to use the rejectNetworkErrorHelper too
| return Promise.reject({ | ||
| status: 404, | ||
| data: {}, | ||
| dev: 'useSelfHostedCurrentUser - 404 schema parsing failed', | ||
| } satisfies NetworkErrorObject) |
There was a problem hiding this comment.
Can we throw in the new rejectNetworkError helper function here 👀
| @@ -2,7 +2,7 @@ import omitBy from 'lodash/omitBy' | |||
| import qs from 'qs' | |||
| import { useHistory, useLocation } from 'react-router-dom' | |||
|
|
|||
| export function useLocationParams(defaultParams = {}) { | |||
| export function useLocationParams(defaultParams: Record<string, unknown> = {}) { | |||
| @@ -2,7 +2,7 @@ import omitBy from 'lodash/omitBy' | |||
| import qs from 'qs' | |||
| import { useHistory, useLocation } from 'react-router-dom' | |||
|
|
|||
| export function useLocationParams(defaultParams = {}) { | |||
| export function useLocationParams(defaultParams: Record<string, unknown> = {}) { | |||
There was a problem hiding this comment.
If it's gonna be a significant investment, we may want to skip it, and wait until we move routers to save time ... and potentially lots of hair pulling.
| createUserToken: z | ||
| .object({ | ||
| error: z | ||
| .union([ |
There was a problem hiding this comment.
One small nit, can we move this to a .discriminatedUnion(), you can read a bit more about them here, but they're a super awesome way to union on a single field and makes checking for said field super easy.
There was a problem hiding this comment.
Gah totally missed that here - sorry you had to point it out both in the other PR and here - will make sure not to miss that next time!
Convert a batch of files to ts
Closes codecov/engineering-team#2720