Skip to content

ref: Refactor doSessionsRequest() & doReleaseHealthRequest() to pass includeAllArgs=true#87675

Merged
ryan953 merged 1 commit intomasterfrom
ryan953/ref-doReleaseHealthRequest-doSessionsRequest
Mar 24, 2025
Merged

ref: Refactor doSessionsRequest() & doReleaseHealthRequest() to pass includeAllArgs=true#87675
ryan953 merged 1 commit intomasterfrom
ryan953/ref-doReleaseHealthRequest-doSessionsRequest

Conversation

@ryan953
Copy link
Member

@ryan953 ryan953 commented Mar 22, 2025

There were a bunch of any types in here, and some janky method signatures because both these methods accepted includeAllArgs:false to be passed in.

Instead of allowing for that flexibility, lets just hard-code includeAllArgs:true and simplify all the types along the way.

Notice some improvements:

  • we're returning ApiResult<...> instead of manually writing out the tuple
  • requestData: any; and requester: any; are gone, meaning we have typesafe params at the site of the method calls
  • Whatever requestData.field is doing, i didn't want to change it too much. injectedFields.push seems pointless; but i'm not breaking it.
  • We can easily unpack the ApiResult tuple with .then(([response]) => {...

@ryan953 ryan953 requested review from a team as code owners March 22, 2025 01:00
@ryan953 ryan953 requested a review from a team March 22, 2025 01:00
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 22, 2025
@ryan953 ryan953 requested a review from a team March 22, 2025 16:06
@ryan953 ryan953 merged commit f04ffe7 into master Mar 24, 2025
41 checks passed
@ryan953 ryan953 deleted the ryan953/ref-doReleaseHealthRequest-doSessionsRequest branch March 24, 2025 17:57
andrewshie-sentry pushed a commit that referenced this pull request Mar 27, 2025
…`includeAllArgs=true` (#87675)

There were a bunch of `any` types in here, and some janky method
signatures because both these methods accepted `includeAllArgs:false` to
be passed in.

Instead of allowing for that flexibility, lets just hard-code
`includeAllArgs:true` and simplify all the types along the way.

Notice some improvements:
- we're returning `ApiResult<...>` instead of manually writing out the
tuple
- `requestData: any;` and `requester: any;` are gone, meaning we have
typesafe params at the site of the method calls
- Whatever `requestData.field` is doing, i didn't want to change it too
much. `injectedFields.push` seems pointless; but i'm not breaking it.
- We can easily unpack the `ApiResult` tuple with `.then(([response]) =>
{...`
@github-actions github-actions bot locked and limited conversation to collaborators Apr 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants