chore(ts): api.jsx to api.tsx by dashed · Pull Request #14148 · getsentry/sentry

dashed · 2019-07-25T19:18:23Z

NOTES

unable to come up with proper types for this.hasProjectBeenRenamed()
unable to come up with a deterministic function signature for error callback.

TODO

rewrite api mocks to TS

src/sentry/static/sentry/app/api.tsx

src/sentry/static/sentry/app/utils/requestError/requestError.tsx

src/sentry/static/sentry/app/api.tsx

dashed · 2019-07-26T01:30:36Z

src/sentry/static/sentry/app/api.tsx

There may be a bug for when includeAllArgs is true here. 🤔

/cc @lynnagara @billyvg @markstory

I'd love to see all the arguments being forwarded by default. Having to remember to set includeAllArgs and also unpack an array to get at the xhr object is more tedious than it needs to be.

yeah -- we'd have to update the requestPromise calls to always expect an array

dashed · 2019-07-26T01:37:05Z

src/sentry/static/sentry/app/api.tsx

this.hasProjectBeenRenamed() expects JQueryXHR as the input; but this cannot be guaranteed.

Introduced in #8799

markstory · 2019-07-26T14:13:02Z

src/sentry/static/sentry/app/api.tsx

+};
+
+type QueryArgs =
+  | {


Should there be a leading | here?

I tried to remove it; but prettier seems to be adding this leading |.

src/sentry/static/sentry/app/api.tsx

markstory · 2019-07-26T14:23:40Z

src/sentry/static/sentry/app/api.tsx

I'd love to see all the arguments being forwarded by default. Having to remember to set includeAllArgs and also unpack an array to get at the xhr object is more tedious than it needs to be.

dashed · 2019-07-30T00:39:19Z

src/sentry/static/sentry/app/api.tsx

+type RequestCallbacks = {
+  success?: (data: any, textStatus?: string, xhr?: JQueryXHR) => void;
+  complete?: (jqXHR: JQueryXHR, textStatus: string) => void;
+  error?: FunctionCallback;


I'm unable to type this properly based on its usage from this file alone. So I'm leaving it as FunctionCallback.

When Sentry is significantly migrated to TS, we can revisit and refine this type.

Should we be marking things like this somehow?

dashed · 2019-07-30T00:41:28Z

src/sentry/static/sentry/app/api.tsx

+    IncludeAllArgsType extends true
+      ? [any, string | undefined, JQueryXHR | undefined]
+      : any
+  > {


@markstory I'm able to add a conditional type so that the right promise type is returned depending on if includeAllArgs is true.

billyvg · 2019-08-02T20:53:33Z

@dashed is this ready for a final review?

dashed · 2019-08-03T00:39:44Z

@billyvg not really no. I'm going to take a few more passes on it and then squash my commits.

dashed · 2019-08-09T17:04:54Z

Looks like travis-ci is happy with this one now.

@billyvg this PR should be ready to go with the mocks typed.

lynnagara

🚢

dashed added the WIP label Jul 25, 2019

dashed self-assigned this Jul 25, 2019

dashed commented Jul 25, 2019

View reviewed changes

src/sentry/static/sentry/app/api.tsx Outdated Show resolved Hide resolved

dashed commented Jul 25, 2019

View reviewed changes

src/sentry/static/sentry/app/utils/requestError/requestError.tsx Outdated Show resolved Hide resolved

dashed commented Jul 26, 2019

View reviewed changes

src/sentry/static/sentry/app/api.tsx Outdated Show resolved Hide resolved

dashed force-pushed the typescript/api-client branch from a6d8429 to 85e12c0 Compare July 26, 2019 01:18

dashed commented Jul 26, 2019

View reviewed changes

src/sentry/static/sentry/app/api.tsx Outdated Show resolved Hide resolved

dashed commented Jul 26, 2019

View reviewed changes

dashed mentioned this pull request Jul 26, 2019

chore(ts): Convert some files/utils useful for api.jsx #14159

Merged

dashed force-pushed the typescript/api-client branch from 44ca1ab to 611cafe Compare July 26, 2019 05:57

markstory reviewed Jul 26, 2019

View reviewed changes

dashed force-pushed the typescript/api-client branch 2 times, most recently from 7867c8f to 742abeb Compare July 29, 2019 22:35

dashed commented Jul 30, 2019

View reviewed changes

dashed marked this pull request as ready for review July 30, 2019 00:41

dashed requested review from a team, lynnagara and markstory July 30, 2019 00:42

dashed removed the WIP label Jul 30, 2019

dashed force-pushed the typescript/api-client branch from 865c659 to 351ed3e Compare July 30, 2019 04:34

dashed added the WIP label Aug 3, 2019

dashed force-pushed the typescript/api-client branch 2 times, most recently from 0636923 to b53ae0a Compare August 8, 2019 20:27

dashed removed the WIP label Aug 9, 2019

dashed requested review from billyvg and evanpurkhiser August 9, 2019 17:04

dashed mentioned this pull request Aug 10, 2019

chore(ts): Initial typescript conversion in preparation of discover v2 #14345

Merged

8 tasks

dashed force-pushed the typescript/api-client branch from 918da0b to 4ce9039 Compare August 12, 2019 19:52

lynnagara approved these changes Aug 13, 2019

View reviewed changes

dashed force-pushed the typescript/api-client branch from ee2656e to 7ed19f9 Compare August 13, 2019 05:44

dashed added 19 commits August 13, 2019 10:44

rename

2a97e53

type api.tsx

de0e5b2

first pass on generics stuffs for wrapCallback

0d7427a

default initializer for Client

260b064

use conditional types

c388672

stricter types

b1a80fc

more type hints

3c17ba9

sdfasf

c95e5f9

people dont like lodash around here

ae2503f

update todo

11ddab1

attempt to type mocks

ef257be

lint fix

9756a42

fix mocks

d5617a5

update all the snapshots

ec0cc05

add Client type to some api variables

eed8a49

rebase whomp

e8ee35c

yarn lock thing

a9f444a

add Client type for incident rule action

4c9e746

lint fix

09128c5

dashed force-pushed the typescript/api-client branch from 7ecf76c to 09128c5 Compare August 13, 2019 14:44

dashed merged commit 9d13ff3 into master Aug 13, 2019

dashed deleted the typescript/api-client branch August 13, 2019 15:10

github-actions bot locked and limited conversation to collaborators Dec 20, 2020

Uh oh!

Conversation

dashed commented Jul 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

NOTES

TODO

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dashed Jul 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

billyvg commented Aug 2, 2019

Uh oh!

dashed commented Aug 3, 2019

Uh oh!

dashed commented Aug 9, 2019

Uh oh!

lynnagara left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dashed commented Jul 25, 2019 •

edited

Loading

dashed Jul 26, 2019 •

edited

Loading