feat: allow ignoring runtime warnings

[paoloricciuti]

Svelte 5 rewrite

This will allow closing #12514 once we ignore the warning in sveltejs/kit#12484

For the moment this is a very ad-hoc ignore handling specifically for this warning (since it's a runtime warning and not every runtime warning can be handled in the same way).

In the sveltekit PR i was asking if it's worth ignoring just this runtime warning or if we should find a more coherent solution but in the meantime i was playing around with this idea and i've implemented it so i thought "Why not open a PR and discuss down there"...so feel free to close if this is not the approach that we want.

Please note that the Svelte codebase is currently being rewritten for Svelte 5. Changes should target Svelte 5, which lives on the default branch (main).

If your PR concerns Svelte 4 (including updates to svelte.dev.docs), please ensure the base branch is svelte-4 and not main.

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests and linting

• Run the tests with pnpm test and lint the project with pnpm lint

[changeset-bot]

🦋 Changeset detected

Latest commit: 2aad265

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
svelte	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Rich-Harris]

I think there are enough use cases for a generic approach to ignoring runtime warnings that we should just do that. A lot more work/code in the short term, but more scalable in the long term

[paoloricciuti]

I think there are enough use cases for a generic approach to ignoring runtime warnings that we should just do that. A lot more work/code in the short term, but more scalable in the long term

Yeah that was my thinking too...do you think the approach should still be with comments in the code? I was also thinking about some svelte:options thing.

[Rich-Harris]

It needs to be contextual, so yes — it should really use the same mechanism from an authoring perspective. The implementation will be tricky though

[dummdidumm]

I don't think we can make all runtime warnings disappear, only a few which have a clear line of code where to disable it. Those are

• state_snapshot_uncloneable
• binding_property_non_reactive
• hydration_attribute_changed
• hydration_html_changed
• ownership_invalid_binding (I think?)
• ownership_invalid_mutation

Of these, the last two are a bit trickier because the validation is baked into the proxy somewhat, while for the others it's likely straightforward to omit adding the validation function or passing a "don't validate" boolean into the corresponding function.

[paoloricciuti]

I don't think we can make all runtime warnings disappear, only a few which have a clear line of code where to disable it. Those are

• state_snapshot_uncloneable
• binding_property_non_reactive
• hydration_attribute_changed
• hydration_html_changed
• ownership_invalid_binding (I think?)
• ownership_invalid_mutation

Of these, the last two are a bit trickier because the validation is baked into the proxy somewhat, while for the others it's likely straightforward to omit adding the validation function or passing a "don't validate" boolean into the corresponding function.

What i was thinking of (dunno if it's a good idea) is to transform svelte ignores like this

<1-- svelte-ignore my_error -->
<input type="text" bind:value={f[0]} />

$.push_ignores('my_error');
var input = root();

$.remove_input_defaults(input);
$.bind_value(input, () => f[0], ($$value) => f[0] = $$value);
$.append($$anchor, input);
$.pop_ignores('my_error');

so that we can access those ignores directly in the warning function and avoid warning. I even started experimenting a bit but for the moment i'm a bit stuck because i can't seem to find a way to do it.

[Rich-Harris]

In theory think that's broadly the right direction, yeah. The 'fun' parts are a) the order in which things are authored doesn't precisely match the order in which generated statements appear in code (i.e. attaching bindings and stuff happens after we've set up text effects and what-have-you), and b) the ignores need to be bound to the effect such that if you push an ignore before an if block that's initially falsy, when it later becomes truthy the ignore still needs to work.

The alternative approach sketched by @dummdidumm is certainly a lot more straightforward to implement, so I'm definitely open to continuing down this PR's current path. It would force the comments to be more local to the thing they're suppressing (i.e. suppressing ownership validation at the point of mutation, rather than e.g. next to an element that happens to contain a component in which the mutation occurs), which could be a blessing or a curse depending on your point of view.

[paoloricciuti]

The alternative approach sketched by @dummdidumm is certainly a lot more straightforward to implement, so I'm definitely open to continuing down this PR's current path.

Ok i will continue working on this PR by adding the various ignores...the only thing i don't like about this approach is that whenever we add a new runtime error we need to also add the code to ignore it and every time might differ a bit. But i guess for the moment is fine. Will work on this as soon as i have time.

[paoloricciuti]

As prophesied by @dummdidumm i've implemented all the runtime ignores but i'm kinda stuck at ownership_invalid_mutation...the warning is thrown inside the set trap so there's no way to pass information to it. I thought of using the ProxyMetadata but that would mean adding the ignore at the declaration level which i don't think it's inline with the rest of the ignores and it's also too wide to be useful.

One idea i have to fix this problem is to conditionally call check_ownership in dev and if there's ignore passing the proxy as parameter so that it can retrieve the metadata. So basically this

import * as $ from "svelte/internal/client";

var on_click = (_, test) => {
-	test(test().test = "stuff", true);
+	test($.check_ownership(test().test = "stuff"), true);
};

var root = $.template(`<button></button>`);

export default function Child($$anchor, $$props) {
	$.push($$props, true);

	let test = $.prop($$props, "test", 7);
	var button = root();

	button.__click = [on_click, test];
	$.append($$anchor, button);
	$.pop();
}

$.delegate(["click"]);

but i'm not so sure if this is even possible I'll call it a day for today so that you guys can also validate my approach with the rest of the warnings...as i've anticipated is a lot of manual work which i don't properly like but it should be functional.

Let me know if i should change something in this approach and if you think the aforementioned solution for the last one could work.

P.s. are we sure that those are the only ignorable runtime warns? For example could state_proxy_equality_mismatch or invalid_raw_snippet_render could be silenced as well (but i guess it doesn't make sense to silence them)

[dummdidumm]

My idea for tackling this: surround the mutation with a function (or one before/after, whichever is easier) which sets a global variable which the ownership check uses to say "ok I shouldn't check right now"

// code in component
skip_ownership(() => test(test().test = "stuff", true));

// code in dev/ownerhip.js
let skip = false;
function skip_ownership(fn) {
  skip = true;
  fn();
  skip = false;
}

// ...
if (!skip) {
  w.ownership_invalid_mutation
}

[paoloricciuti]

My idea for tackling this: surround the mutation with a function (or one before/after, whichever is easier) which sets a global variable which the ownership check uses to say "ok I shouldn't check right now"

// code in component
skip_ownership(() => test(test().test = "stuff", true));

// code in dev/ownerhip.js
let skip = false;
function skip_ownership(fn) {
  skip = true;
  fn();
  skip = false;
}

// ...
if (!skip) {
  w.ownership_invalid_mutation
}

Oh much better than my idea...will do this evening

[paoloricciuti]

@Rich-Harris @dummdidumm with the last commit it should be it, all the runtime warning should be skippable...i think i covered every possible case for the ownership invalid mutation...also again i hope the general approach is good but let me know if we need any changes and i'll update.

[Rich-Harris]

This mostly LGTM but I notice we're sometimes adding the additional argument in prod, i.e. this...

// svelte-ignore state_snapshot_uncloneable
let snapshot = $state.snapshot(x);

...becomes this whether in dev or prod, even though the second argument is unused in prod:

let snapshot = $.snapshot(x, true);

We could be disciplined about only doing it in dev, or passing state into is_ignored, but that's tedious. I think a better solution would be to take advantage of the state.js mechanism we introduced recently. I'll open a separate PR for that so we can use it here

[paoloricciuti]

I was about to PR the ignore to sveltekit...but now that i think about it this will only prevent the warning with versions above the next release and if we add the ignore it will actually double the amount of warning...is that fine or should we check the exact version before ignoring?

[dummdidumm]

I'd say it's fine - this is still prerelease mode, and people who update their SvelteKit package will also update their Svelte package. The PR / changeset should probably mention that though.