Skip to content

Commit

Permalink
improve mount/unmount handling
Browse files Browse the repository at this point in the history
  • Loading branch information
@edmundhung
edmundhung committed Dec 11, 2024
1 parent 268d324 commit 29ff8e0
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 59 deletions.
29 changes: 12 additions & 17 deletions packages/conform-dom/dom.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,9 @@ export function requestSubmit(
export function syncFieldValue(
element: FieldElement,
value: unknown,
updateDefaultValue: boolean,
defaultValue: unknown,
): void {
const fieldValue =
const getInputValue = (value: unknown): string[] =>
typeof value === 'string'
? [value]
: Array.isArray(value) && value.every((item) => typeof item === 'string')
Expand All @@ -154,27 +154,22 @@ export function syncFieldValue(

if (element instanceof HTMLSelectElement) {
for (const option of element.options) {
option.selected = fieldValue.includes(option.value);

if (updateDefaultValue) {
option.defaultSelected = option.selected;
}
option.selected = getInputValue(value).includes(option.value);
option.defaultSelected = getInputValue(defaultValue).includes(
option.value,
);
}
} else if (
element instanceof HTMLInputElement &&
(element.type === 'checkbox' || element.type === 'radio')
) {
element.checked = fieldValue.includes(element.value);

if (updateDefaultValue) {
element.defaultChecked = element.checked;
}
element.checked = getInputValue(value).includes(element.value);
element.defaultChecked = getInputValue(defaultValue).includes(
element.value,
);
} else {
element.value = fieldValue[0] ?? '';

if (updateDefaultValue) {
element.defaultValue = element.value;
}
element.value = getInputValue(value)[0] ?? '';
element.defaultValue = getInputValue(defaultValue)[0] ?? '';
}
}

Expand Down
5 changes: 1 addition & 4 deletions packages/conform-dom/form.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ export type Constraint = {

export type FormMeta<FormError> = {
formId: string;
isResetting: boolean;
isValueUpdated: boolean;
submissionStatus?: 'error' | 'success';
defaultValue: Record<string, unknown>;
Expand Down Expand Up @@ -272,7 +271,6 @@ function createFormMeta<Schema, FormError, FormValue>(
const initialValue = lastResult?.initialValue ?? defaultValue;
const result: FormMeta<FormError> = {
formId: options.formId,
isResetting: !!isResetting,
isValueUpdated: false,
submissionStatus: lastResult?.status,
defaultValue,
Expand Down Expand Up @@ -671,7 +669,6 @@ export function createFormContext<

return {
submissionStatus: next.submissionStatus,
isResetting: next.isResetting,
defaultValue,
initialValue,
value,
Expand Down Expand Up @@ -1091,7 +1088,7 @@ export function syncFormState<FormError>(
syncFieldValue(
element,
stateSnapshot.initialValue[element.name],
isInitializing || stateSnapshot.isResetting,
stateSnapshot.defaultValue[element.name],
);
}
}
Expand Down
60 changes: 42 additions & 18 deletions playground/app/routes/sync-form-state.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,21 @@ import { parseWithZod } from '@conform-to/zod';
import type { ActionFunctionArgs } from '@remix-run/node';
import { json } from '@remix-run/node';
import { Form, useActionData } from '@remix-run/react';
import { useState } from 'react';
import { z } from 'zod';
import { Playground, Field } from '~/components';

const schema = z.object({
text: z.string({ required_error: 'Text is required' }),
textarea: z.string({ required_error: 'Textarea is required' }),
select: z.string({ required_error: 'Select is required' }),
multiSelect: z.array(z.string(), {
required_error: 'Multi select is required',
input: z.object({
text: z.string(),
number: z.number(),
}),
checkbox: z.boolean({ required_error: 'Checkbox is required' }),
checkboxGroup: z.array(z.string(), {
required_error: 'Checkbox group is required',
}),
radioGroup: z.string({ required_error: 'Radio is required' }),
textarea: z.string(),
select: z.string(),
multiSelect: z.array(z.string()),
checkbox: z.boolean(),
checkboxGroup: z.array(z.string()),
radioGroup: z.string(),
});

export async function action({ request }: ActionFunctionArgs) {
Expand All @@ -29,7 +29,10 @@ export async function action({ request }: ActionFunctionArgs) {
resetForm: true,
}),
defaultValue: {
text: 'Default text',
input: {
text: 'Default text',
number: 4,
},
textarea: 'You need to write something here',
select: 'red',
multiSelect: ['apple', 'banana', 'cherry'],
Expand All @@ -47,7 +50,10 @@ export default function Example() {
shouldValidate: 'onBlur',
onValidate: ({ formData }) => parseWithZod(formData, { schema }),
defaultValue: actionData?.defaultValue ?? {
text: 'Hello World',
input: {
text: 'Hello World',
number: 2,
},
textarea: 'Once upon a time',
select: 'green',
multiSelect: ['banana', 'cherry'],
Expand All @@ -56,11 +62,13 @@ export default function Example() {
radioGroup: 'Deutsch',
},
constraint: {
text: {
'input.text': {
required: true,
minLength: 5,
maxLength: 30,
pattern: '[a-zA-Z]+',
},
'input.number': {
min: 5,
max: 10,
step: 1,
Expand All @@ -74,7 +82,6 @@ export default function Example() {
required: true,
},
multiSelect: {
required: true,
multiple: true,
},
checkbox: {
Expand All @@ -91,16 +98,23 @@ export default function Example() {
return element.name !== 'token';
},
});
const inputFields = fields.input.getFieldset();
const [showNumberField, setShowNumberField] = useState(true);

return (
<Form method="post" {...getFormProps(form)}>
<Playground title="Sync form state" result={actionData?.lastResult}>
<Field label="Token">
<input name="token" defaultValue="1-0624770" />
</Field>
<Field label="Text" meta={fields.text}>
<input name={fields.text.name} />
<Field label="Text" meta={inputFields.text}>
<input name={inputFields.text.name} />
</Field>
{showNumberField ? (
<Field label="Number" meta={inputFields.number}>
<input type="number" name={inputFields.number.name} />
</Field>
) : null}
<Field label="Textarea" meta={fields.textarea}>
<textarea name={fields.textarea.name} />
</Field>
Expand All @@ -121,7 +135,7 @@ export default function Example() {
<Field label="Checkbox" meta={fields.checkbox}>
<label className="inline-block">
<input type="checkbox" name={fields.checkbox.name} />
<span className="p-2">It works</span>
<span className="p-2">Show number field</span>
</label>
</Field>
<Field label="Checkbox group" meta={fields.checkboxGroup}>
Expand All @@ -147,7 +161,10 @@ export default function Example() {
<button
{...form.update.getButtonProps({
value: {
text: 'Updated',
input: {
text: 'Updated',
number: 3,
},
textarea: 'Some text here',
select: 'blue',
multiSelect: ['apple', 'cherry'],
Expand All @@ -159,6 +176,13 @@ export default function Example() {
>
Update value
</button>
<hr />
<button
type="button"
onClick={() => setShowNumberField((prev) => !prev)}
>
Toggle number field
</button>
</Playground>
</Form>
);
Expand Down
69 changes: 49 additions & 20 deletions tests/integrations/sync-form-state.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import { getPlayground } from './helpers';
function getFieldset(form: Locator) {
return {
token: form.locator('[name="token"]'),
text: form.locator('[name="text"]'),
text: form.locator('[name="input.text"]'),
number: form.locator('[name="input.number"]'),
textarea: form.locator('[name="textarea"]'),
select: form.locator('[name="select"]'),
multiSelect: form.locator('[name="multiSelect"]'),
Expand All @@ -15,9 +16,12 @@ function getFieldset(form: Locator) {
}

async function assertFieldsetValue(
form: Locator,
fieldset: ReturnType<typeof getFieldset>,
value: {
text: string;
input: {
text: string;
number: number;
};
textarea: string;
select: string;
multiSelect: string[];
Expand All @@ -26,12 +30,11 @@ async function assertFieldsetValue(
radioGroup: string;
},
) {
const fieldset = getFieldset(form);

// This check if Conform omit the token field based on the shouldSyncElement option
await expect(fieldset.token).toHaveValue('1-0624770');

await expect(fieldset.text).toHaveValue(value.text);
await expect(fieldset.text).toHaveValue(value.input.text);
await expect(fieldset.number).toHaveValue(value.input.number.toString());
await expect(fieldset.textarea).toHaveValue(value.textarea);
await expect(fieldset.select).toHaveValue(value.select);
await expect(fieldset.multiSelect).toHaveValues(value.multiSelect);
Expand Down Expand Up @@ -63,11 +66,18 @@ test('sync fields value', async ({ page }) => {
await page.goto('/sync-form-state');

const playground = getPlayground(page);
const fieldset = getFieldset(playground.container);
const updateValueButton = playground.container.locator(
'button:text("Update value")',
);
const toggleNumberFieldButton = playground.container.locator(
'button:text("Toggle number field")',
);
const defaultValue = {
text: 'Hello World',
input: {
text: 'Hello World',
number: 2,
},
textarea: 'Once upon a time',
select: 'green',
multiSelect: ['banana', 'cherry'],
Expand All @@ -76,7 +86,10 @@ test('sync fields value', async ({ page }) => {
radioGroup: 'Deutsch',
};
const valueToBeSet = {
text: 'Updated',
input: {
text: 'Updated',
number: 3,
},
textarea: 'Some text here',
select: 'blue',
multiSelect: ['apple', 'cherry'],
Expand All @@ -85,7 +98,10 @@ test('sync fields value', async ({ page }) => {
radioGroup: 'English',
};
const newDefaultValue = {
text: 'Default text',
input: {
text: 'Default text',
number: 4,
},
textarea: 'You need to write something here',
select: 'red',
multiSelect: ['apple', 'banana', 'cherry'],
Expand All @@ -94,19 +110,30 @@ test('sync fields value', async ({ page }) => {
radioGroup: 'Français',
};

await assertFieldsetValue(playground.container, defaultValue);
await assertFieldsetValue(fieldset, defaultValue);
await updateValueButton.click();
await assertFieldsetValue(playground.container, valueToBeSet);
await assertFieldsetValue(fieldset, valueToBeSet);

// Check if mounting/unmounting the number field will break the sync
await toggleNumberFieldButton.click();
await expect(fieldset.number).not.toBeAttached();
await toggleNumberFieldButton.click();
// FIXME: The value of the number field should be the default value?
// We need a better way to differentiate default value vs last submitted value vs value user set
await expect(fieldset.number).toHaveValue(
valueToBeSet.input.number.toString(),
);

await playground.reset.click();
await assertFieldsetValue(playground.container, defaultValue);
await assertFieldsetValue(fieldset, defaultValue);
await updateValueButton.click();
await assertFieldsetValue(playground.container, valueToBeSet);
await assertFieldsetValue(fieldset, valueToBeSet);
await playground.submit.click();
await assertFieldsetValue(playground.container, newDefaultValue);
await assertFieldsetValue(fieldset, newDefaultValue);
await updateValueButton.click();
await assertFieldsetValue(playground.container, valueToBeSet);
await assertFieldsetValue(fieldset, valueToBeSet);
await playground.reset.click();
await assertFieldsetValue(playground.container, newDefaultValue);
await assertFieldsetValue(fieldset, newDefaultValue);
});

test('sync validation attributes', async ({ page }) => {
Expand All @@ -120,16 +147,18 @@ test('sync validation attributes', async ({ page }) => {
await expect(fieldset.text).toHaveJSProperty('maxLength', 30);
await expect(fieldset.text).toHaveJSProperty('pattern', '[a-zA-Z]+');
await expect(fieldset.text).toHaveJSProperty('multiple', false);
await expect(fieldset.text).toHaveJSProperty('min', '5');
await expect(fieldset.text).toHaveJSProperty('max', '10');
await expect(fieldset.text).toHaveJSProperty('step', '1');

await expect(fieldset.number).toHaveJSProperty('required', false);
await expect(fieldset.number).toHaveJSProperty('min', '5');
await expect(fieldset.number).toHaveJSProperty('max', '10');
await expect(fieldset.number).toHaveJSProperty('step', '1');

await expect(fieldset.textarea).toHaveJSProperty('required', true);
await expect(fieldset.textarea).toHaveJSProperty('minLength', 10);
await expect(fieldset.textarea).toHaveJSProperty('maxLength', 1000);

await expect(fieldset.select).toHaveJSProperty('required', true);
await expect(fieldset.multiSelect).toHaveJSProperty('required', true);
await expect(fieldset.multiSelect).toHaveJSProperty('required', false);
await expect(fieldset.multiSelect).toHaveJSProperty('multiple', true);

await expect(fieldset.checkbox).toHaveJSProperty('required', true);
Expand Down

0 comments on commit 29ff8e0

Please sign in to comment.