feat(js): Add useApi hook and implement in withApi #28257
Conversation
| } | ||
| }; | ||
| return WithApi; | ||
| }; |
There was a problem hiding this comment.
Probably easier to read this by just looking at the file
| wrapper.unmount(); | ||
|
|
||
| expect(apiInstance?.clear).toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
I took a couple tests away because the useApi tests covers them
static/app/utils/useApi.tsx
Outdated
| * component is unmounted. | ||
| * | ||
| * This may be useful in situations where your component needs to finish up | ||
| * some where the client was passed into some type of action creator and the |
There was a problem hiding this comment.
| * some where the client was passed into some type of action creator and the | |
| * somewhere the client was passed into some type of action creator and the |
| type Props = { | ||
| /** | ||
| * Test passthrough API clients | ||
| */ | ||
| api?: Client; | ||
| /** | ||
| * Test persistInFlight | ||
| */ | ||
| persistInFlight?: boolean; | ||
| }; | ||
|
|
||
| const MyComponent: React.FC<Props> = ({api, persistInFlight}) => { | ||
| apiInstance = useApi({api, persistInFlight}); | ||
| return <div />; | ||
| }; |
There was a problem hiding this comment.
not sure.. but should we move this out of describe?
There was a problem hiding this comment.
I'm indifferent. I think we usually have stuff in describe blocks
| }; | ||
|
|
||
| const MyComponent: React.FC<Props> = ({api, persistInFlight}) => { | ||
| apiInstance = useApi({api, persistInFlight}); |
There was a problem hiding this comment.
imo it's important to test this hook, but it feels like an implementation detail
There was a problem hiding this comment.
I disagree, I think this is a proper test that the api client is provided to the hook caller.
How else would you test a hook like this?
There was a problem hiding this comment.
I understand and agree with you... I was just thinking that api is an implementation detail that we have to test anyway, but it would be an exception on purpose of the RTL
There was a problem hiding this comment.
Oh sure, I see what you're saying. Yeah
evanpurkhiser
force-pushed
the
evanpurkhiser/feat-js-add-useapi-hook-and-implement-in-withapi
branch
from
4c1aa61 to
013fdfb
Compare
| } | ||
|
|
||
| // Use the provided client if available | ||
| const api = providedApi ?? localApi.current!; |
There was a problem hiding this comment.
Yes unfortunately typescript isn't smart enough to understand that we set it above
evanpurkhiser
deleted the
evanpurkhiser/feat-js-add-useapi-hook-and-implement-in-withapi
branch
Introduces a useApi hook that can be used in place of wrapping components with
withApi