-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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 to TypeScript 4.4 #2322
Comments
@benmccann you may already know this, but for now rollup builds get stuck with typescript 4.4 or greater, making this milestone dependant on this upstream issue: rollup/plugins#983 |
Thanks. I wasn't aware, but expect we won't face that issue because we don't use |
Vite does. I don't know whether it's just part of its own build process or whether the shipped package ends up containing a bundled copy of it. |
I still imagine it shouldn't matter for us. We don't ship the TypeScript library. It will just enable slightly stricter type checking on our own code when we run |
I've just tried to work on this issue and I think I got it to work, but I'm not sure what path to take around handling a breaking change: After upgrading, all the new TypeScript compilation errors were caused by the new
My solution so far took path 3, creating a utility Edit: Removed |
That sounds reasonable to me. @ignatiusmb @dummdidumm are more of TypeScript experts than I am though |
Btw, we also have a |
Sounds reasonable to me, but we need to be careful that these are all |
I think it's also worth noting that
I tried this as well but in a typescript file, and typescript actually explicitly prevents this pattern (throws error), e.g. NOTE: vite-plugin-svelte disables |
You're correct, catch bindings can only be typed as
I worried non-Error values could be thrown too, but relaxed a little after all tests ran without triggering the assertion errors. Now, investigating further, I found one instance of throwing/rejecting a non-Error value and two catch bindings expecting its shape (1, 2). Maybe I should create a Otherwise, is there some additional way to test if it's safe to implement the checks as
As is recommended by the linked article, I made sure |
I think the safer solution would be create such a In general I'm a little hesitant to use option 3 just because such an error could by anything and we can only make assumptions about its structure given the code paths we see.
I don't think there's a good way to test this, the best way I can think of is to check the code paths that lead to these errors. |
Yeah sounds good if Unrelated: If you do make a PR, make sure internal functions are snake_case (https://github.com/sveltejs/kit#coding-style) |
Describe the problem
We're on TypeScript 4.3
Describe the proposed solution
Upgrade to 4.4
Alternatives considered
No response
Importance
nice to have
Additional Information
No response
The text was updated successfully, but these errors were encountered: