Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "fix: wrap form control in flush sync" #513

Merged
merged 1 commit into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions packages/conform-dom/form.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,6 @@ export type FormOptions<Schema, FormError = string[], FormValue = Schema> = {
submitter: HTMLInputElement | HTMLButtonElement | null;
formData: FormData;
}) => Submission<Schema, FormError, FormValue>;

/**
* To schedule when an intent should be dispatched.
*/
onSchedule?: (callback: () => void) => void;
};

export type SubscriptionSubject = {
Expand Down Expand Up @@ -971,10 +966,11 @@ export function createFormContext<
}

function createFormControl<Type extends Intent['type']>(type: Type) {
const schedule =
latestOptions.onSchedule ?? ((callback: () => void) => callback());
const control = (payload: any = {}) =>
schedule(() => dispatch({ type, payload }));
dispatch({
type,
payload,
});

return Object.assign(control, {
getButtonProps(payload: any = {}) {
Expand Down
9 changes: 2 additions & 7 deletions packages/conform-react/hooks.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { type FormId, type FieldName } from '@conform-to/dom';
import { useEffect, useId, useState, useLayoutEffect } from 'react';
import { flushSync } from 'react-dom';
import {
type FormMetadata,
type FieldMetadata,
Expand Down Expand Up @@ -50,7 +49,7 @@ export function useForm<
FormError = string[],
>(
options: Pretty<
Omit<FormOptions<Schema, FormError, FormValue>, 'formId' | 'onSchedule'> & {
Omit<FormOptions<Schema, FormError, FormValue>, 'formId'> & {
/**
* The form id. If not provided, a random id will be generated.
*/
Expand All @@ -71,11 +70,7 @@ export function useForm<
const { id, ...formConfig } = options;
const formId = useFormId<Schema, FormError>(id);
const [context] = useState(() =>
createFormContext({
...formConfig,
onSchedule: (callback: () => void) => flushSync(callback),
formId,
}),
createFormContext({ ...formConfig, formId }),
);

useSafeLayoutEffect(() => {
Expand Down
6 changes: 2 additions & 4 deletions packages/conform-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,11 @@
"@conform-to/dom": "1.0.3"
},
"devDependencies": {
"@types/react": "^18.2.64",
"@types/react-dom": "^18.2.20",
"@types/react": "^18.2.43",
"react": "^18.2.0"
},
"peerDependencies": {
"react": ">=18",
"react-dom": ">=18"
"react": ">=18"
},
"keywords": [
"constraint-validation",
Expand Down
2 changes: 1 addition & 1 deletion playground/app/components.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ interface PlaygroundProps {
title: string;
description?: ReactNode;
form?: string;
result?: Record<string, unknown> | null | undefined;
result?: Record<string, unknown>;
formAction?: string;
formMethod?: string;
formEncType?: string;
Expand Down
17 changes: 1 addition & 16 deletions playground/app/routes/form-control.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export default function FormControl() {
<FormProvider context={form.context}>
<Form method="post" {...getFormProps(form)}>
<FormStateInput formId={form.id} />
<Playground title="Form Control" result={form.value}>
<Playground title="Form Control" result={lastResult}>
<Field label="Name" meta={fields.name}>
<input {...getInputProps(fields.name, { type: 'text' })} />
</Field>
Expand Down Expand Up @@ -115,21 +115,6 @@ export default function FormControl() {
>
Reset form
</button>
<button
className="rounded-md border p-2 hover:border-black"
type="button"
onClick={() => {
form.update({
name: fields.message.name,
value: 'Hello World',
});
form.reset({
name: fields.number.name,
});
}}
>
Reset number with message updated
</button>
</div>
</Playground>
</Form>
Expand Down
31 changes: 3 additions & 28 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 4 additions & 28 deletions tests/integrations/form-control.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,10 @@ function getFieldset(form: Locator) {
clearMessage: form.locator('button:text("Clear message")'),
resetMessage: form.locator('button:text("Reset message")'),
resetForm: form.locator('button:text("Reset form")'),
resetNumberWithMessageUpdated: form.locator(
'button:text("Reset number with message updated")',
),
};
}

async function runTest(
page: Page,
options: {
clientValidate?: boolean;
} = {},
) {
async function runValidationScenario(page: Page) {
const playground = getPlayground(page);
const fieldset = getFieldset(playground.container);

Expand Down Expand Up @@ -63,33 +55,17 @@ async function runTest(
await expect(fieldset.number).toHaveValue('');
await expect(fieldset.message).toHaveValue('');
await expect(playground.error).toHaveText(['', '', '']);

if (options.clientValidate) {
await fieldset.number.fill('123');
await expect.poll(playground.result).toStrictEqual({
number: '123',
});

await fieldset.resetNumberWithMessageUpdated.click();
await expect(fieldset.number).toHaveValue('');
await expect(fieldset.message).toHaveValue('Hello World');
await expect.poll(playground.result).toStrictEqual({
message: 'Hello World',
});
}
}

test.describe('With JS', () => {
test('Client Validation', async ({ page }) => {
await page.goto('/form-control');
await runTest(page, {
clientValidate: true,
});
await runValidationScenario(page);
});

test('Server Validation', async ({ page }) => {
await page.goto('/form-control?noClientValidate=yes');
await runTest(page);
await runValidationScenario(page);
});
});

Expand All @@ -98,6 +74,6 @@ test.describe('No JS', () => {

test('Validation', async ({ page }) => {
await page.goto('/form-control');
await runTest(page);
await runValidationScenario(page);
});
});
Loading