fix: inject false for unchecked boolean checkboxes in remote form submission - #15786
fix: inject false for unchecked boolean checkboxes in remote form submission#15786PranavAgarkar07 wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 39e3570 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 |
08e2d70 to
d1edd02
Compare
|
Good catch, applied. Thanks! |
d1edd02 to
39e3570
Compare
|
Unfortunately this can't work because it introduces a difference in behavior between enhanced and non-enhanced forms. If there's no JavaScript, the false value won't be injected. |
| event.preventDefault(); | ||
|
|
||
| const form_data = new FormData(form, event.submitter); | ||
| const form_data = get_form_data(form, event.submitter); |
There was a problem hiding this comment.
ottomated is right. We probably need to account for undefined checked values on the server rather than modifying the data on the client so that it works without JS on the client
| await expect(page.locator('#set-value-display')).toHaveText('Set via method'); | ||
| }); | ||
|
|
||
| test('unchecked checkbox submits successfully', async ({ page }) => { |
There was a problem hiding this comment.
We should probably move this test to test.js so that it's tested without JS too
|
Makes sense — preventing non-optional booleans at the schema level is the right fix since it works consistently with and without JS. Closing in favor of #15804. Thanks for the feedback @ottomated @teemingc! |
…in their form schemas (#15804) closes #15785 closes #15786 Through typescript trickery, prevent form schemas from containing non-optional booleans, i.e.: ```ts // Not allowed: form(v.object({ checkbox: v.boolean(), })); // Allowed: form(v.object({ checkbox: v.optional(v.boolean(), false) })); ``` This prevents the pitfall of forgetting that `<input type="checkbox">` elements don't send a false value when they're unchecked. --- ### Please don't delete this checklist! Before submitting the PR, please make sure you do the following: - [x] 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 - [x] This message body should clearly illustrate what problems it solves. - [x] Ideally, include a test that fails without this PR but passes with it. ### Tests - [x] Run the tests with `pnpm test` and lint the project with `pnpm lint` and `pnpm check` ### Changesets - [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running `pnpm changeset` and following the prompts. Changesets that add features should be `minor` and those that fix bugs should be `patch`. Please prefix changeset messages with `feat:`, `fix:`, or `chore:`. ### Edits - [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed. --------- Co-authored-by: Rich Harris <rich.harris@vercel.com>
Fixes #15785.
In SvelteKit 2.59.0, unchecked checkboxes using
z.coerce.boolean()failed to submit because the client-side form submission logic didn't account forb:-prefixed fields missing fromFormData.This PR adds a
get_form_datahelper inform.svelte.jsthat injects'false'into theFormDatafor anyb:-prefixed input that is absent from theFormData(i.e. unchecked checkboxes).A regression test has been added to
packages/kit/test/apps/asyncto verify that unchecked checkboxes submit successfully.