Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade typescript and fix unknown ypes #1787

Merged
merged 2 commits into from
Sep 7, 2021
Merged

Conversation

kr8n3r
Copy link
Contributor

@kr8n3r kr8n3r commented Aug 31, 2021

What

In Typescript 4.4.2 there is a breaking change https://devblogs.microsoft.com/typescript/announcing-typescript-4-4/#using-unknown-in-catch-variables

Our acceptance tests that run on staging and prod fail.

This adds type any to variable in the try/catch block.

more info:

How to review

  • if you have a dev env, deploy there and run the acceptance test
  • alternatively, obtain envs from cf and credhub, export variable to point to dev01 or dev02 and run npm run test:acceptance

Who can review

not @kr8n3r


🚨⚠️ Please do not merge this pull request via the GitHub UI ⚠️🚨

kr8n3r added 2 commits August 31, 2021 11:43
In Typescript 4.4.2 there is a breaking change https://devblogs.microsoft.com/typescript/announcing-typescript-4-4/#using-unknown-in-catch-variables

Our acceptance tests that run on staging and prod fail.

This adds type any to variable in the try/catch block
@kr8n3r
Copy link
Contributor Author

kr8n3r commented Sep 1, 2021

@paroxp what do you think of changes in 48079c9 ?

as we can't do catch ( err: AxiosError) this is the best i could come up to check that type is AxiosError

Screenshot 2021-09-01 at 14 09 53

nevermind, doesn't work

@paroxp
Copy link
Member

paroxp commented Sep 1, 2021

Yeah, this looks good. I imagine you still need any or unknown type. And if I think about it, there could be number of errors that could be thrown. Us doing the checks and resolving to default behaviour if it's not something we expected is OK I think.

@kr8n3r
Copy link
Contributor Author

kr8n3r commented Sep 1, 2021

Yeah, this looks good. I imagine you still need any or unknown type. And if I think about it, there could be number of errors that could be thrown. Us doing the checks and resolving to default behaviour if it's not something we expected is OK I think.

unknown is now the default with typescript 4.4.2 and --strict

as tests are now failing will have to look at a different solution, might have to be something so I can do
err instance of <customErrorThatExtendsAxiosError>

for reasons I don't know request here doesn't result in an AxiosError

@kr8n3r kr8n3r force-pushed the ugrade-typescript-fix-types branch from af7b11a to e604b99 Compare September 6, 2021 07:24
@kr8n3r
Copy link
Contributor Author

kr8n3r commented Sep 6, 2021

@paroxp going back to previous any type for the error as it's become to cumbersome

@kr8n3r kr8n3r merged commit 2a3d5a0 into main Sep 7, 2021
@kr8n3r kr8n3r deleted the ugrade-typescript-fix-types branch September 7, 2021 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants