feat: TypeScript 3.9 upgrade #7324
Conversation
|
Would we be breaking the published types by doing this for people who haven't upgraded TS in their projects to 3.9? |
|
@Conduitry if the type definitions exposed by the Svelte repository (stores etc.) would only contain valid TypeScript
Since there are no real big changes made to the syntax, the risk is tolerable in my opinion. |
|
According to latest package-lock.json, already we are using 3.9.10. Therefore I think we can update this now without regarding it as breaking change. |
|
I think we bumped it at some point for something internal and said "we gotta be careful not to introduce anything that's not 3.x compatible which is the minimum version we actually support. Not sure what the real minimum version is though. But if this change only changes things internally but not externally, yes we could merge it. |
|
We're going to be doing a Svelte 4 release soon with some housekeeping. We might as well upgrade to the latest TypeScript as part of it if you'd like to update this |
|
@ivanhofer is attempting to deploy a commit to the Svelte Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
@benmccann I'm done with the merge.
There are some TODO's left where I would need some help.
| @@ -50,6 +50,7 @@ export function invalidate(renderer: Renderer, scope: Scope, node: Node, names: | |||
| const extra_args = tail.map(variable => get_invalidated(variable)).filter(Boolean); | |||
|
|
|||
| if (is_store_value) { | |||
| // TODO: check why there are 4 parameters, but `set_store_value` only expects 3 | |||
| return x`@set_store_value(${head.name.slice(1)}, ${node}, ${head.name}, ${extra_args})`; | |||
There was a problem hiding this comment.
Can someone please confirm that it is wrong to pass 4 arguments?
| if (store == null) { | ||
| return noop; | ||
| } | ||
| const unsub = store.subscribe(...callbacks); | ||
| // TODO: `store` does not accept an array of callbacks | ||
| const unsub: any = store.subscribe(...(callbacks as [any])); |
There was a problem hiding this comment.
I don't understand what happens here. Can someone please help?
src/runtime/internal/utils.ts
Outdated
| component.$$.on_destroy.push(subscribe(store, callback)); | ||
| } | ||
|
|
||
| export function create_slot(definition, ctx, $$scope, fn) { | ||
| // TODO: find out what types are expected here instead of using `any` everywhere |
| // 1. should be improved or | ||
| // 2. where casting is needed in order to satisfy TypeScript | ||
| // a deeper look at these parts is needed to check if they can be replaced with a normal cast or if they currently contain a potential bug | ||
| type TODO<T = any> = T |
There was a problem hiding this comment.
Most things were typed after enabling TypeScript's strict mode.
Those TODO casts are not always needed currently, but I think enabling strict mode for the future makes sense.
Before submitting the PR, please make sure you do the following
[feat],[fix],[chore], or[docs].Tests
npm testand lint the project withnpm run lintThe content of this PR originally belonged to #7224 that introduces a "more complex" TypeScript type.
TLDR: the changes of this PR are not required for #7224 but would improve the Svelte codebase.
(and I think I have found one or the oder "bug" while adding types)
3.9.xstrictCompiler option set to true`There are still some type definitions missing and a few open TODOs.
I have opened this PR as a draft to be able to sl´plit the original PR.
Note: I have created a type called
TODOwhich marks parts that need a deeper look either because the type-definitions need to be improved or it currently is unclear (to me) what the actual type should be.Step 4 + 5 could be eliminated if the type definitions of
createEventDispatcherare moved to a separate file.