From 323a492ce378d80d073976745c7cd54acbc46c22 Mon Sep 17 00:00:00 2001 From: Edmund Hung Date: Wed, 29 Mar 2023 00:33:24 +0200 Subject: [PATCH] fix(conform-react): defaultValue should not be cached (#130) --- packages/conform-react/hooks.ts | 179 +++++++----------- playground/app/components.tsx | 2 +- playground/app/routes/reset-default-value.tsx | 104 ++++++++++ .../integrations/reset-default-value.spec.ts | 47 +++++ 4 files changed, 217 insertions(+), 115 deletions(-) create mode 100644 playground/app/routes/reset-default-value.tsx create mode 100644 tests/integrations/reset-default-value.spec.ts diff --git a/packages/conform-react/hooks.ts b/packages/conform-react/hooks.ts index 1c787e7b..dfe7dd53 100644 --- a/packages/conform-react/hooks.ts +++ b/packages/conform-react/hooks.ts @@ -157,44 +157,39 @@ export function useForm< return ([] as string[]).concat(config.lastSubmission.error['']); }); - const [uncontrolledState, setUncontrolledState] = useState< - FieldsetConfig - >(() => { + const initialError = useMemo(() => { const submission = config.lastSubmission; if (!submission) { - return { - defaultValue: config.defaultValue, - }; + return {}; } const scope = getScope(submission.intent); - return { - defaultValue: submission.payload as FieldValue | undefined, - initialError: Object.entries(submission.error).reduce< - Record - >((result, [name, message]) => { - if (name !== '' && (scope === null || scope === name)) { - result[name] = message; - } + return Object.entries(submission.error).reduce< + Record + >((result, [name, message]) => { + if (name !== '' && (scope === null || scope === name)) { + result[name] = message; + } - return result; - }, {}), - }; - }); - const fieldsetConfig = { - ...uncontrolledState, + return result; + }, {}); + }, [config.lastSubmission]); + const ref = config.ref ?? formRef; + const fieldset = useFieldset(ref, { + defaultValue: + (config.lastSubmission?.payload as FieldValue) ?? + config.defaultValue, + initialError, constraint: config.constraint, form: config.id, - }; - const ref = config.ref ?? formRef; - const fieldset = useFieldset(ref, fieldsetConfig); + }); const [noValidate, setNoValidate] = useState( config.noValidate || !config.fallbackNative, ); - useEffect(() => { + useSafeLayoutEffect(() => { configRef.current = config; }); @@ -288,7 +283,6 @@ export function useForm< }; const handleReset = (event: Event) => { const form = ref.current; - const formConfig = configRef.current; if (!form || event.target !== form) { return; @@ -303,18 +297,8 @@ export function useForm< } setErrors([]); - setUncontrolledState({ - defaultValue: formConfig.defaultValue, - }); }; - /** - * The input event handler will be triggered in capturing phase in order to - * allow follow-up action in the bubble phase based on the latest validity - - * E.g. `useFieldset` reset the error of valid field after checking the - * validity in the bubble phase. - */ document.addEventListener('input', handleInput, true); document.addEventListener('blur', handleBlur, true); document.addEventListener('invalid', handleInvalid, true); @@ -466,53 +450,29 @@ export function useFieldset>( config: FieldsetConfig | FieldConfig, ): Fieldset { const configRef = useRef(config); - const [uncontrolledState, setUncontrolledState] = useState<{ - defaultValue: FieldValue; - initialError: Record | undefined>; - }>( - // @ts-expect-error + const [error, setError] = useState>( () => { - const initialError: Record< - string, - Record | undefined - > = {}; - - for (const [name, message] of Object.entries( - config?.initialError ?? {}, - )) { - const [key, ...paths] = getPaths(name); - - if (typeof key === 'string') { - const scopedName = getName(paths); + const initialError = config?.initialError; - initialError[key] = { - ...initialError[key], - [scopedName]: message, - }; - } + if (!initialError) { + return {}; } - return { - defaultValue: config?.defaultValue ?? {}, - initialError, - }; - }, - ); - const [error, setError] = useState>( - () => { const result: Record = {}; - for (const [key, error] of Object.entries( - uncontrolledState.initialError, - )) { - result[key] = ([] as string[]).concat(error?.[''] ?? []); + for (const [name, message] of Object.entries(initialError)) { + const [key, ...paths] = getPaths(name); + + if (typeof key === 'string' && paths.length === 0) { + result[key] = ([] as string[]).concat(message ?? []); + } } return result; }, ); - useEffect(() => { + useSafeLayoutEffect(() => { configRef.current = config; }); @@ -564,15 +524,6 @@ export function useFieldset>( return; } - const fieldsetConfig = configRef.current as - | FieldsetConfig - | undefined; - - setUncontrolledState({ - // @ts-expect-error - defaultValue: fieldsetConfig?.defaultValue ?? {}, - initialError: {}, - }); setError({}); }; @@ -599,14 +550,25 @@ export function useFieldset>( return; } - const fieldsetConfig = (config ?? {}) as FieldsetConfig; + const fieldsetConfig = config as FieldsetConfig; const constraint = fieldsetConfig.constraint?.[key]; const errors = error?.[key]; + const initialError = Object.entries( + fieldsetConfig.initialError ?? {}, + ).reduce((result, [name, message]) => { + const [field, ...paths] = getPaths(name); + + if (field === key) { + result[getName(paths)] = message; + } + + return result; + }, {} as Record); const field: FieldConfig = { ...constraint, name: fieldsetConfig.name ? `${fieldsetConfig.name}.${key}` : key, - defaultValue: uncontrolledState.defaultValue[key], - initialError: uncontrolledState.initialError[key], + defaultValue: fieldsetConfig.defaultValue?.[key], + initialError, error: errors?.[0], errors, }; @@ -634,41 +596,24 @@ export function useFieldList( config: FieldConfig>, ): Array<{ key: string } & FieldConfig> { const configRef = useRef(config); - const [uncontrolledState, setUncontrolledState] = useState<{ - defaultValue: FieldValue>; - initialError: Array | undefined>; - }>(() => { - const initialError: Array | undefined> = - []; + const [error, setError] = useState(() => { + const initialError: Array = []; for (const [name, message] of Object.entries(config?.initialError ?? {})) { const [index, ...paths] = getPaths(name); - if (typeof index === 'number') { - const scopedName = getName(paths); - - initialError[index] = { - ...initialError[index], - [scopedName]: message, - }; + if (typeof index === 'number' && paths.length === 0) { + initialError[index] = ([] as string[]).concat(message ?? []); } } - return { - defaultValue: config.defaultValue ?? [], - initialError, - }; + return initialError; }); - const [error, setError] = useState>(() => - uncontrolledState.initialError.map((error) => - ([] as string[]).concat(error?.[''] ?? []), - ), - ); const [entries, setEntries] = useState< Array<[string, FieldValue | undefined]> >(() => Object.entries(config.defaultValue ?? [undefined])); - useEffect(() => { + useSafeLayoutEffect(() => { configRef.current = config; }); @@ -774,13 +719,7 @@ export function useFieldList( return; } - const fieldConfig = configRef.current; - - setUncontrolledState({ - defaultValue: fieldConfig.defaultValue ?? [], - initialError: [], - }); - setEntries(Object.entries(fieldConfig.defaultValue ?? [undefined])); + setEntries(Object.entries(configRef.current.defaultValue ?? [undefined])); setError([]); }; @@ -799,10 +738,22 @@ export function useFieldList( return entries.map(([key, defaultValue], index) => { const errors = error[index]; + const initialError = Object.entries(config.initialError ?? {}).reduce( + (result, [name, message]) => { + const [field, ...paths] = getPaths(name); + + if (field === index) { + result[getName(paths)] = message; + } + + return result; + }, + {} as Record, + ); const fieldConfig: FieldConfig = { name: `${config.name}[${index}]`, - defaultValue: defaultValue ?? uncontrolledState.defaultValue[index], - initialError: uncontrolledState.initialError[index], + defaultValue: defaultValue ?? config.defaultValue?.[index], + initialError, error: errors?.[0], errors, }; diff --git a/playground/app/components.tsx b/playground/app/components.tsx index 89012fcd..71d28b52 100644 --- a/playground/app/components.tsx +++ b/playground/app/components.tsx @@ -4,7 +4,7 @@ import { useEffect, useState } from 'react'; interface PlaygroundProps { title: string; - description?: string; + description?: ReactNode; form?: string; lastSubmission?: Submission; formAction?: string; diff --git a/playground/app/routes/reset-default-value.tsx b/playground/app/routes/reset-default-value.tsx new file mode 100644 index 00000000..37904a0b --- /dev/null +++ b/playground/app/routes/reset-default-value.tsx @@ -0,0 +1,104 @@ +import { useForm, conform } from '@conform-to/react'; +import { parse } from '@conform-to/zod'; +import type { ActionArgs } from '@remix-run/node'; +import { json } from '@remix-run/node'; +import { Form, Link, useActionData, useLoaderData } from '@remix-run/react'; +import { z } from 'zod'; +import { Field, Playground } from '~/components'; + +const schema = z.object({ + name: z.string().min(1, 'Name is required'), + code: z.string().regex(/^#[A-Fa-f0-9]{6}$/, 'The code is invalid'), +}); + +const color: Record | undefined> = { + red: { + name: 'Red', + code: '#FF0000', + }, + green: { + name: 'Green', + code: '#00FF00', + }, + blue: { + name: 'Blue', + code: '#0000FF', + }, +}; + +export let loader = async ({ request }: ActionArgs) => { + const url = new URL(request.url); + + return json({ + defaultValue: color[url.searchParams.get('color') ?? ''], + }); +}; + +export let action = async ({ request }: ActionArgs) => { + const formData = await request.formData(); + const submission = parse(formData, { + schema, + }); + + if (!submission.value || submission.intent !== 'submit') { + return json(submission); + } + + throw new Error('Not implemented'); +}; + +export default function TodoForm() { + const { defaultValue } = useLoaderData(); + const lastSubmission = useActionData(); + const [form, fieldset] = useForm>({ + defaultValue, + lastSubmission, + onValidate({ formData }) { + return parse(formData, { schema }); + }, + }); + + return ( +
+ + Please choose a color +
    +
  • + + Red + +
    • +
  • + + Green + +
    • +
  • + + Blue + +
    • +
+ + } + lastSubmission={lastSubmission} + > + + + + + + + +
+ ); +} diff --git a/tests/integrations/reset-default-value.spec.ts b/tests/integrations/reset-default-value.spec.ts new file mode 100644 index 00000000..47cdf9ba --- /dev/null +++ b/tests/integrations/reset-default-value.spec.ts @@ -0,0 +1,47 @@ +import { type Page, type Locator, test, expect } from '@playwright/test'; +import { getPlayground } from '../helpers'; + +function getFieldset(form: Locator) { + return { + name: form.locator('[name="name"]'), + code: form.locator('[name="code"]'), + }; +} + +async function runValidationScenario(page: Page) { + const playground = getPlayground(page); + const fieldset = getFieldset(playground.container); + + await playground.container.getByText('Red').click(); + + await expect(fieldset.name).toHaveValue('Red'); + await expect(fieldset.code).toHaveValue('#ff0000'); + + await playground.container.getByText('Green').click(); + + await expect(fieldset.name).toHaveValue('Green'); + await expect(fieldset.code).toHaveValue('#00ff00'); + + await playground.container.getByText('Blue').click(); + + await expect(fieldset.name).toHaveValue('Blue'); + await expect(fieldset.code).toHaveValue('#0000ff'); +} + +test.beforeEach(async ({ page }) => { + await page.goto('/reset-default-value'); +}); + +test.describe('With JS', () => { + test('Validation', async ({ page }) => { + await runValidationScenario(page); + }); +}); + +test.describe('No JS', () => { + test.use({ javaScriptEnabled: false }); + + test('Validation', async ({ page }) => { + await runValidationScenario(page); + }); +});