From a3ebacc8e2213be2f5b26ffabd662a1c60a898b3 Mon Sep 17 00:00:00 2001 From: Daniel Weinmann Date: Thu, 23 Feb 2023 07:11:28 -0300 Subject: [PATCH] Clear global errors on new submission --- .../examples/actions/environment.spec.ts | 4 +- .../examples/actions/global-error.spec.ts | 10 +- apps/web/tests/setup/example.ts | 6 ++ packages/remix-forms/src/createForm.tsx | 91 +++++++++++++------ 4 files changed, 73 insertions(+), 38 deletions(-) diff --git a/apps/web/tests/examples/actions/environment.spec.ts b/apps/web/tests/examples/actions/environment.spec.ts index c0ece6d6..616e7e71 100644 --- a/apps/web/tests/examples/actions/environment.spec.ts +++ b/apps/web/tests/examples/actions/environment.spec.ts @@ -74,9 +74,7 @@ testWithoutJS('With JS disabled', async ({ example }) => { // Submit form await button.click() await page.reload() - await expect(page.locator('form > div[role="alert"]:visible')).toHaveText( - 'Missing custom header', - ) + await example.expectGlobalError('Missing custom header') // Submit with valid headers await page.setExtraHTTPHeaders({ customHeader: 'foo' }) diff --git a/apps/web/tests/examples/actions/global-error.spec.ts b/apps/web/tests/examples/actions/global-error.spec.ts index dbe08804..5d16a52e 100644 --- a/apps/web/tests/examples/actions/global-error.spec.ts +++ b/apps/web/tests/examples/actions/global-error.spec.ts @@ -42,7 +42,8 @@ test('With JS enabled', async ({ example }) => { await example.expectValid(password) // Submit form - button.click() + await example.expectNoGlobalError() + await button.click() await expect(button).toBeDisabled() // Show global error @@ -50,7 +51,8 @@ test('With JS enabled', async ({ example }) => { // Submit valid form await password.input.fill('supersafe') - button.click() + await button.click() + await example.expectNoGlobalError() await example.expectData({ email: 'john@doe.com', password: 'supersafe' }) }) @@ -101,9 +103,7 @@ testWithoutJS('With JS disabled', async ({ example }) => { await page.reload() // Show global error - await expect(page.locator('form > div[role="alert"]:visible')).toHaveText( - 'Wrong email or password', - ) + await example.expectGlobalError('Wrong email or password') // Submit valid form await password.input.fill('supersafe') diff --git a/apps/web/tests/setup/example.ts b/apps/web/tests/setup/example.ts index 779bd149..4d889ae5 100644 --- a/apps/web/tests/setup/example.ts +++ b/apps/web/tests/setup/example.ts @@ -129,6 +129,12 @@ class Example { ).toHaveText(message) } + async expectNoGlobalError() { + expect( + await this.page.locator('form > div[role="alert"]:visible').count(), + ).toEqual(0) + } + async expectErrorMessage(fieldName: string, message: string) { await expect( this.page.locator(`#errors-for-${fieldName}:visible`), diff --git a/packages/remix-forms/src/createForm.tsx b/packages/remix-forms/src/createForm.tsx index e27a4e6a..78bbb963 100644 --- a/packages/remix-forms/src/createForm.tsx +++ b/packages/remix-forms/src/createForm.tsx @@ -223,8 +223,16 @@ function createForm({ const actionErrors = actionData?.errors as FormErrors const actionValues = actionData?.values as FormValues - const errors = { ...errorsProp, ...actionErrors } - const values = { ...valuesProp, ...actionValues } + + const errors = React.useMemo( + () => ({ ...errorsProp, ...actionErrors }), + [errorsProp, actionErrors], + ) + + const values = React.useMemo( + () => ({ ...valuesProp, ...actionValues }), + [valuesProp, actionValues], + ) const schemaShape = objectFromSchema(schema).shape const defaultValues = mapObject(schemaShape, (key, fieldShape) => { @@ -292,10 +300,14 @@ function createForm({ ], ) - const fieldErrors = (key: keyof SchemaType) => { - const message = (formErrors[key] as unknown as FieldError)?.message - return browser() ? message && [message] : errors && errors[key] - } + const fieldErrors = React.useCallback( + (key: keyof SchemaType) => { + const message = (formErrors[key] as unknown as FieldError)?.message + return browser() ? message && [message] : errors && errors[key] + }, + [errors, formErrors], + ) + const firstErroredField = () => Object.keys(schemaShape).find((key) => fieldErrors(key)?.length) const makeField = (key: string) => { @@ -333,34 +345,45 @@ function createForm({ } as Field } - const hiddenFieldsErrorsToGlobal = (globalErrors: string[] = []) => { - const deepHiddenFieldsErrors = hiddenFields?.map((hiddenField) => { - const hiddenFieldErrors = fieldErrors(hiddenField) - - if (hiddenFieldErrors instanceof Array) { - const hiddenFieldLabel = - (labels && labels[hiddenField]) || inferLabel(String(hiddenField)) - return hiddenFieldErrors.map( - (error) => `${hiddenFieldLabel}: ${error}`, - ) - } else return [] - }) - const hiddenFieldsErrors: string[] = deepHiddenFieldsErrors?.flat() || [] + const hiddenFieldsErrorsToGlobal = React.useCallback( + (globalErrors: string[] = []) => { + const deepHiddenFieldsErrors = hiddenFields?.map((hiddenField) => { + const hiddenFieldErrors = fieldErrors(hiddenField) + + if (hiddenFieldErrors instanceof Array) { + const hiddenFieldLabel = + (labels && labels[hiddenField]) || inferLabel(String(hiddenField)) + return hiddenFieldErrors.map( + (error) => `${hiddenFieldLabel}: ${error}`, + ) + } else return [] + }) + const hiddenFieldsErrors: string[] = + deepHiddenFieldsErrors?.flat() || [] - const allGlobalErrors = ([] as string[]) - .concat(globalErrors, hiddenFieldsErrors) - .filter((error) => typeof error === 'string') + const allGlobalErrors = ([] as string[]) + .concat(globalErrors, hiddenFieldsErrors) + .filter((error) => typeof error === 'string') - return allGlobalErrors.length > 0 ? allGlobalErrors : undefined - } + return allGlobalErrors.length > 0 ? allGlobalErrors : undefined + }, + [fieldErrors, hiddenFields, labels], + ) - let globalErrors = hiddenFieldsErrorsToGlobal(errors?._global) + const globalErrors = React.useMemo( + () => hiddenFieldsErrorsToGlobal(errors?._global), + [errors?._global, hiddenFieldsErrorsToGlobal], + ) const buttonLabel = transition.state === 'submitting' ? pendingButtonLabel : rawButtonLabel const [disabled, setDisabled] = React.useState(false) + const [globalErrorsState, setGlobalErrorsState] = React.useState< + string[] | undefined + >(globalErrors) + const customChildren = mapChildren( childrenFn?.({ Field, @@ -403,9 +426,9 @@ function createForm({ autoFocus, }) } else if (child.type === Errors) { - if (!child.props.children && !globalErrors?.length) return null + if (!child.props.children && !globalErrorsState?.length) return null - if (child.props.children || !globalErrors?.length) { + if (child.props.children || !globalErrorsState?.length) { return React.cloneElement(child, { role: 'alert', ...child.props, @@ -414,7 +437,7 @@ function createForm({ return React.cloneElement(child, { role: 'alert', - children: globalErrors.map((error) => ( + children: globalErrorsState.map((error) => ( {error} )), ...child.props, @@ -437,9 +460,9 @@ function createForm({ {Object.keys(schemaShape) .map(makeField) .map((field) => renderField({ Field, ...field }))} - {globalErrors?.length && ( + {globalErrorsState?.length && ( - {globalErrors.map((error) => ( + {globalErrorsState.map((error) => ( {error} ))} @@ -486,8 +509,16 @@ function createForm({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [errorsProp, unparsedActionData]) + React.useEffect(() => { + setGlobalErrorsState(globalErrors) + }, [globalErrors]) + React.useEffect(() => { onTransition && onTransition(form) + + if (transition.state === 'submitting') { + setGlobalErrorsState(undefined) + } // eslint-disable-next-line react-hooks/exhaustive-deps }, [transition.state])