Conversation
🦋 Changeset detectedLatest commit: 02af8c7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
Install the latest version of pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/02af8c735560a8fd4785aa8359c23ab6780f9b80Open in Note This PR is from a fork. A maintainer must approve approve each commit before it can be built and installed. |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
teemingc
left a comment
There was a problem hiding this comment.
Thank you for the PR. Sorry I saw #17131 before this one and prematurely merged it.
I don't think this is the right fix. I feel that it should be fixed in Svelte instead. With this example it's a little surprising for Svelte to unselect the currently selected option.
@teemingc Thank you for reviewing the PR and I understand what your meaning that it should be fixed in Svelte instead. Looking at the history, this is the third time the same pattern has come up:
So I'll open issue and PR to sveltejs/svelte about And when merged and released these PRs in sveltejs/svelte, then I'll bump the By the way, I'm not AI 😆 |
|
I raised the issue with a few other maintainers and it seems that the majority believe Svelte is behaving correctly here. I’m wondering if there’s a way to merge this but without setting every function property as |
|
@teemingc yes configurable is removal. I added I'll push that changes, thanks |
… yet
`field.as('select')` always spread `value` onto the `<select>`, even while the
field has no value (the control was never touched). Svelte re-applies a spread
`value` to a `<select>` whenever any spread prop changes — for example
`aria-invalid` after `validate()` — and an `undefined` value clears the
selection, so an untouched `<select>` ended up blank with `selectedIndex -1`.
Leave `value` out of the spread until the field has a value so the browser
keeps the current selection, in both the "unrelated field validated" case and
the "this field's own issues changed" case.
Fixes sveltejs#17131
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PcqPEahv26ddHVqhPZHTJw
…ssion test Give the text field a rule the test violates so `validate()` has a visible signal, and cover the issue both appearing and clearing. Also make the unit assertion self-contained and document why `add_props` getters are configurable. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PcqPEahv26ddHVqhPZHTJw
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PcqPEahv26ddHVqhPZHTJw
93f996d to
da6ab70
Compare
<select> that has no value yet<select> selected when validation runs
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PcqPEahv26ddHVqhPZHTJw
|
@teemingc Sorry to ping you. Can you review again? |
Fixes #17131
What happens
An untouched
<select {...field.as('select')}>goes blank (selectedIndex -1) as soon asvalidate()runs, for instance from anonchangehandler on a neighbouring<input>. Once blank, the control no longer contributes toFormData, so arequiredschema then reportsInvalid key: Expected "picked" but received undefinedinstead of the intended message.Root cause
field.as('select')always spreads avalueprop, and thatvalueisundefineduntil the field has a value, becauseinputstarts out empty.valueto the<select>, and a post-mountundefinedthat matches no option deselects everything.aria-invalidreads the wholeissuesobject, so everyvalidate()/submit()re-runs the spread. That is how the issue's scenario triggers it. The same happens when the<select>gets or loses its own issue, becausearia-invalidreally changes then.The deselect in (2) is intended Svelte behaviour (see
binding-select-unmatchedin the Svelte test suite, and the discussion in this PR), so the fix belongs in Kit.Fix
Leave
valueout of the spread while the field has no value, so the browser keeps the current selection. As soon as the field gets a value (user picks an option,field.set(...), fallback, reset),valueappears in the spread and Svelte applies it as before.This uses a small proxy (
omit_while_undefined) that hides the key from spreads. Only the select'svaluegetter is madeconfigurable, which the proxy needs in order to hide it;add_propsis unchanged.Scope: this only covers the never-set state. If a value is set and later cleared with
field.set(undefined), the select is deselected, exactly as before this PR. Usefield.set('')to go back to an<option value="">placeholder.#17132 took a different approach (making
issuesgranular so unrelated validation doesn't re-run the spread). It was merged and then reverted in #17184, because the select still went blank when its ownaria-invalidchanged.Tests
remote/form/select-untouched-validateand two Playwright tests in the async app: an unrelated field validated (its issue appearing and clearing again), and the select's own issues changing. Both fail onversion-3(selectedIndexis-1) and pass with this PR, in dev and build mode. The existing regression test for fix: granular updates offield.value()#14621 is kept.form-utils.spec.js:as('select')omitsvaluewhile the field has none and includes it once it has one.<select>renders blank whenvalidate()runs ononchange#17131 (.for(uid),optional(string(), ''),onchange={() => validate()}), including a select without an empty-value option: selection is kept through repeated validation, user selection and reset.form-utils.spec.js: 76 passed. Async appremote functions(dev): 67 passed, 8 skipped.pnpm lint,pnpm -F @sveltejs/kit check.Screenshots
Untouched
<select>after typing in the text field, blurring it (validate()), and thenvalidate({ all: true }).Before: the select is blank and the schema can't even see the field.
After:
-- pick --stays selected and the field's own issue is reported.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits
🤖 Generated with Claude Code
https://claude.ai/code/session_01PcqPEahv26ddHVqhPZHTJw