fix: update value like attributes in a separate template_effect

[paoloricciuti]

Svelte 5 rewrite

Closes #11718

template_effects for values that the user can change outside of the reactivity (like value or checked) needs to be isolated or else an unrelated change could reset the input.

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: 411065b

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

[trueadm]

It would be good to have a test like the original case and also one where we spread the value in too.

[dummdidumm]

I think this is the correct fix - the value attribute is "uncontrolled" and therefore can get out of sync, so it needs its own render effect. Dominic is right that we also should have a spreading test to guard against regressions (it does work fine without adjustments, we just need a test).

[paoloricciuti]

It would be good to have a test like the original case and also one where we spread the value in too.

The original case is covered by the legacy test but i can add it anyway

I think this is the correct fix - the value attribute is "uncontrolled" and therefore can get out of sync, so it needs its own render effect. Dominic is right that we also should have a spreading test to guard against regressions (it does work fine without adjustments, we just need a test).

Sure...adding it right now

[dummdidumm]

The original case is covered by the legacy test but i can add it anyway

I think it's fine to not add another test, as you say the legacy test covers it

[paoloricciuti]

The original case is covered by the legacy test but i can add it anyway

I think it's fine to not add another test, as you say the legacy test covers it

I just added the spread test for runtime-runes is that enough or is better to have it also in legacy?

[paoloricciuti]

Mmm testing the test for spread in the current playground i see that is still working despite being in a single template_effect...maybe we could avoid splitting in case of spread to speed things up?

$.template_effect(() => {
	attributes = $.set_attributes(input, attributes, { type: "text", ...value }, true, "");
	attributes_1 = $.set_attributes(textarea, attributes_1, { ...value }, true, "");
	attributes_2 = $.set_attributes(input_1, attributes_2, { type: "checkbox", ...checked }, true, "");
	$.set_text(text, $.get(count));
});

it's not completely clear to me why it is working even if it's updating the attributes again but maybe there's some check in set_attributes?

[trueadm]

@paoloricciuti Ah that could be because are using our internal property cache on the DOM element. I wonder if we can re-use that for values too actually? Instead of having a separate template effect, maybe we can create a function that sets the value and also uses that internal property. I'll have a look into that now.

[trueadm]

Implemented an alternative that avoids needing to make extra effects #11726

[paoloricciuti]

@paoloricciuti Ah that could be because are using our internal property cache on the DOM element. I wonder if we can re-use that for values too actually? Instead of having a separate template effect, maybe we can create a function that sets the value and also uses that internal property. I'll have a look into that now.

Uh i wasn't aware of that internal cache...your solution is much better 🎉