Conversation
# Conflicts: # static/app/views/feedback/feedbackListPage.tsx
github-actions
Bot
added
the
Scope: Frontend
| * Defaults to path '/organizations/$organizationIdOrSlug/issues/' | ||
| */ | ||
| endpointPath?: string; | ||
| endpoint?: | ||
| | { | ||
| path: '/organizations/$organizationIdOrSlug/issues/'; | ||
| } | ||
| | { | ||
| path: '/organizations/$organizationIdOrSlug/releases/$version/resolved/'; | ||
| version: string; | ||
| }; |
There was a problem hiding this comment.
note: passing raw paths in is impossible with apiOptions. There are only 2 urls that should be requested so I’ve made a union type of objects out of which we can internally create apiOptions.
I would’ve passed in full apiOptions from the call-sides but queryParams are computed here and extracting that is even more complicated
|
|
||
| <TableWrapper> | ||
| <GroupList | ||
| endpointPath={path} |
There was a problem hiding this comment.
removed because that’s just the default value that doesn’t need to be passed
| pageLinks: responsePageLinks, | ||
| rawData: finalRawData, | ||
| }; | ||
| })(); |
There was a problem hiding this comment.
not sure why we had an IIFE here, pretty pointless imo, so removed.
| enabled, | ||
| retry: hasQueueFeature | ||
| ? false | ||
| : (failureCount, error) => { |
There was a problem hiding this comment.
we also get type inference back for retry
| export function useFeedbackApiOptions() { | ||
| const context = useContext(FeedbackApiOptionsProvider); | ||
|
|
||
| if (!context) { | ||
| throw new Error( | ||
| 'useFeedbackApiOptions must be used within a FeedbackApiOptionsProvider' | ||
| ); |
There was a problem hiding this comment.
note: we only ever read from this context when underneath a provider. having an “empty default context” not only hides mistakes when forgetting the provider, it also makes it hard to work with, as apiOptions can’t easily be undefined
now we init the context with null and error out if the Provider is missing, making the mistake explicit and all the usages of the context easier to reason about.
| return apiOptions.asInfinite<FeedbackIssueListItem[]>()( | ||
| '/organizations/$organizationIdOrSlug/issues/', | ||
| { | ||
| path: fixedQueryView ? {organizationIdOrSlug: orgSlug} : skipToken, | ||
| query: fixedQueryView ? buildQuery({fixedQueryView, mailbox, prefetch}) : undefined, | ||
| staleTime: 0, | ||
| } |
There was a problem hiding this comment.
note: we previously returned undefined here when fixedQueryView was not defined. Working with undefined queryOptions is hard though, so disabling it with skipToken feels like the better approach.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 8c06905. Configure here.
2 participants
No description provided.