-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
chore:Update for TS7 #16485
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
base: main
Are you sure you want to change the base?
chore:Update for TS7 #16485
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'svelte': patch | ||
--- | ||
|
||
Update for TS7 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ export default function check_graph_for_cycles(edges) { | |
}, new Map()); | ||
|
||
const visited = new Set(); | ||
/** @type {Set<T>} */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
const on_stack = new Set(); | ||
/** @type {Array<Array<T>>} */ | ||
const cycles = []; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -160,10 +160,14 @@ export function createEventDispatcher() { | |
e.lifecycle_outside_component('createEventDispatcher'); | ||
} | ||
|
||
/** | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same: signature-to-signature checking (like when you're checking that a returned function has the right type) is stricter in TS7, so you have to explicitly note that I also improved the cast on type but I can't remember if that's actually necessary. |
||
* @param [detail] | ||
* @param [options] | ||
*/ | ||
return (type, detail, options) => { | ||
const events = /** @type {Record<string, Function | Function[]>} */ ( | ||
active_component_context.s.$$events | ||
)?.[/** @type {any} */ (type)]; | ||
)?.[/** @type {string} */ (type)]; | ||
|
||
if (events) { | ||
const callbacks = is_array(events) ? events.slice() : [events]; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -64,7 +64,7 @@ export function hmr(original, get_source) { | |
// @ts-expect-error | ||
wrapper[FILENAME] = original[FILENAME]; | ||
|
||
// @ts-expect-error | ||
// @ts-ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error being ignored here is bogus, a result of wonky JS checking that mistakenly causes a circularity. In TS7, this bug is fixed. If this were a complete upgrade to TS7, I would remove this line, but since I want it to compile on TS5 and TS7, I switched it to ts-ignore. |
||
wrapper[HMR] = { | ||
// When we accept an update, we set the original source to the new component | ||
original, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,8 @@ const PENDING = 0; | |
const THEN = 1; | ||
const CATCH = 2; | ||
|
||
/** @typedef {typeof PENDING | typeof THEN | typeof CATCH} AwaitState */ | ||
|
||
/** | ||
* @template V | ||
* @param {TemplateNode} node | ||
|
@@ -67,9 +69,8 @@ export function await_block(node, get_input, pending_fn, then_fn, catch_fn) { | |
: mutable_source(/** @type {V} */ (undefined), false, false); | ||
var error_source = runes ? source(undefined) : mutable_source(undefined, false, false); | ||
var resolved = false; | ||
|
||
/** | ||
* @param {PENDING | THEN | CATCH} state | ||
* @param {AwaitState} state | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here; |
||
* @param {boolean} restore | ||
*/ | ||
function update(state, restore) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -191,7 +191,7 @@ export function each(node, flags, get_collection, get_key, render_fn, fallback_f | |
// store a reference to the effect so that we can update the start/end nodes in reconciliation | ||
each_effect ??= /** @type {Effect} */ (active_effect); | ||
|
||
array = get(each_array); | ||
array = /** @type {V[]} */ (get(each_array)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
var length = array.length; | ||
|
||
if (was_empty && length === 0) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,9 @@ | ||
/** @import { Equals } from '#client' */ | ||
|
||
/** @type {Equals} */ | ||
export function equals(value) { | ||
export const equals = function (value) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sigh, this is actually one of the few places I liked JSDoc more than TS -- the fact that there's no way to type FWIW this actually is going to negatively impact us -- in our language tools, whenever you There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's possible to implement this feature in the right way but it's invasive (all functions have to have a property for the type, even in TS where there's no syntax for it). If I know a real customer is relying on it then maybe we should consider keeping it. @jakebailey and @DanielRosenwasser love the feature but the old implementation was such a bug farm and the usual use of it so redundant with contextual typing that I didn't think it was needed. I'm glad this guinea pig upgrade is turning up problems! I think one workaround would be to split There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I think the most realistic solution we've thrown around in Discord is this transform: export function load(args) {}
// becomes
export function load(args: LoadArgs): LoadReturn {} it's just a lot more parsing and inserting for us 😆 |
||
return value === this.v; | ||
} | ||
}; | ||
|
||
/** | ||
* @param {unknown} a | ||
|
@@ -26,6 +26,6 @@ export function not_equal(a, b) { | |
} | ||
|
||
/** @type {Equals} */ | ||
export function safe_equals(value) { | ||
export const safe_equals = function (value) { | ||
return !safe_not_equal(value, this.v); | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -184,8 +184,7 @@ export function legacy_rest_props(props, exclude) { | |
* The proxy handler for spread props. Handles the incoming array of props | ||
* that looks like `() => { dynamic: props }, { static: prop }, ..` and wraps | ||
* them so that the whole thing is passed to the component as the `$$props` argument. | ||
* @template {Record<string | symbol, unknown>} T | ||
* @type {ProxyHandler<{ props: Array<T | (() => T)> }>}} | ||
* @type {ProxyHandler<{ props: Array<Record<string | symbol, unknown> | (() => Record<string | symbol, unknown>)> }>}} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
*/ | ||
const spread_props_handler = { | ||
get(target, key) { | ||
|
@@ -362,22 +361,23 @@ export function prop(props, key, flags, fallback) { | |
// means we can just call `$$props.foo = value` directly | ||
if (setter) { | ||
var legacy_parent = props.$$legacy; | ||
|
||
return function (/** @type {any} */ value, /** @type {boolean} */ mutation) { | ||
if (arguments.length > 0) { | ||
// We don't want to notify if the value was mutated and the parent is in runes mode. | ||
// In that case the state proxy (if it exists) should take care of the notification. | ||
// If the parent is not in runes mode, we need to notify on mutation, too, that the prop | ||
// has changed because the parent will not be able to detect the change otherwise. | ||
if (!runes || !mutation || legacy_parent || is_store_sub) { | ||
/** @type {Function} */ (setter)(mutation ? getter() : value); | ||
return /** @type {() => V} */ ( | ||
function (/** @type {V} */ value, /** @type {boolean} */ mutation) { | ||
if (arguments.length > 0) { | ||
// We don't want to notify if the value was mutated and the parent is in runes mode. | ||
// In that case the state proxy (if it exists) should take care of the notification. | ||
// If the parent is not in runes mode, we need to notify on mutation, too, that the prop | ||
// has changed because the parent will not be able to detect the change otherwise. | ||
if (!runes || !mutation || legacy_parent || is_store_sub) { | ||
/** @type {Function} */ (setter)(mutation ? getter() : value); | ||
} | ||
|
||
return value; | ||
} | ||
|
||
return value; | ||
return getter(); | ||
} | ||
|
||
return getter(); | ||
}; | ||
); | ||
} | ||
|
||
// Either prop is written to, but there's no binding, which means we | ||
|
@@ -400,29 +400,31 @@ export function prop(props, key, flags, fallback) { | |
|
||
var parent_effect = /** @type {Effect} */ (active_effect); | ||
|
||
return function (/** @type {any} */ value, /** @type {boolean} */ mutation) { | ||
if (arguments.length > 0) { | ||
const new_value = mutation ? get(d) : runes && bindable ? proxy(value) : value; | ||
return /** @type {() => V} */ ( | ||
function (/** @type {any} */ value, /** @type {boolean} */ mutation) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. after running prettier, Hide Whitespace is basically required to review this PR |
||
if (arguments.length > 0) { | ||
const new_value = mutation ? get(d) : runes && bindable ? proxy(value) : value; | ||
|
||
set(d, new_value); | ||
overridden = true; | ||
|
||
set(d, new_value); | ||
overridden = true; | ||
if (fallback_value !== undefined) { | ||
fallback_value = new_value; | ||
} | ||
|
||
if (fallback_value !== undefined) { | ||
fallback_value = new_value; | ||
return value; | ||
} | ||
|
||
return value; | ||
} | ||
// special case — avoid recalculating the derived if we're in a | ||
// teardown function and the prop was overridden locally, or the | ||
// component was already destroyed (this latter part is necessary | ||
// because `bind:this` can read props after the component has | ||
// been destroyed. TODO simplify `bind:this` | ||
if ((is_destroying_effect && overridden) || (parent_effect.f & DESTROYED) !== 0) { | ||
return d.v; | ||
} | ||
|
||
// special case — avoid recalculating the derived if we're in a | ||
// teardown function and the prop was overridden locally, or the | ||
// component was already destroyed (this latter part is necessary | ||
// because `bind:this` can read props after the component has | ||
// been destroyed. TODO simplify `bind:this` | ||
if ((is_destroying_effect && overridden) || (parent_effect.f & DESTROYED) !== 0) { | ||
return d.v; | ||
return get(d); | ||
} | ||
|
||
return get(d); | ||
}; | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -284,7 +284,8 @@ export function update_reaction(reaction) { | |
|
||
try { | ||
reaction.f |= REACTION_IS_UPDATING; | ||
var result = /** @type {Function} */ (0, reaction.fn)(); | ||
var fn = /** @type {Function} */ (reaction.fn); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typescript errors on |
||
var result = fn(); | ||
var deps = reaction.deps; | ||
|
||
if (new_deps !== null) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,7 +35,7 @@ export function validate_store(store, name) { | |
} | ||
|
||
/** | ||
* @template {() => unknown} T | ||
* @template {(...args: any[]) => unknown} T | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. JS arity checking between signatures is lax in TS5 so it didn't catch this. |
||
* @param {T} fn | ||
*/ | ||
export function prevent_snippet_stringification(fn) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JS type annotations no longer insert
typeof
in places where it finds a value but expects a type. You have to do it explicitly, like in TS.