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

fix: revert auto field value update #778

Merged
merged 3 commits into from
Sep 19, 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
10 changes: 10 additions & 0 deletions .changeset/brown-bikes-raise.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"@conform-to/dom": patch
"@conform-to/react": patch
---

fix: revert auto field value update

Revert #729 and #766

The auto field value update feature introduced in v1.2.0 has caused several critical issues with significant user impact. While I appreciate what they accomplished, I’ve realized the current solution isn't robust enough to handle all potential use cases. To minimize the impact on everyone, I believe it's best to revert these changes for now.
36 changes: 18 additions & 18 deletions packages/conform-dom/form.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,7 @@ export type FormValue<Schema> = Schema extends
: Schema extends Array<infer Item>
? string | Array<FormValue<Item>> | undefined
: Schema extends Record<string, any>
?
| { [Key in keyof Schema]?: FormValue<Schema[Key]> }
| undefined
? { [Key in keyof Schema]?: FormValue<Schema[Key]> } | undefined
: unknown;

const error = Symbol('error');
Expand Down Expand Up @@ -278,7 +276,12 @@ function createFormMeta<Schema, FormError, FormValue>(
value: initialValue,
constraint: options.constraint ?? {},
validated: lastResult?.state?.validated ?? {},
key: getDefaultKey(defaultValue),
key: !initialized
? getDefaultKey(defaultValue)
: {
'': generateId(),
...getDefaultKey(defaultValue),
},
// The `lastResult` should comes from the server which we won't expect the error to be null
// We can consider adding a warning if it happens
error: (lastResult?.error as Record<string, FormError>) ?? {},
Expand All @@ -295,20 +298,15 @@ function getDefaultKey(
): Record<string, string> {
return Object.entries(flatten(defaultValue, { prefix })).reduce<
Record<string, string>
>(
(result, [key, value]) => {
if (Array.isArray(value)) {
for (let i = 0; i < value.length; i++) {
result[formatName(key, i)] = generateId();
}
>((result, [key, value]) => {
if (Array.isArray(value)) {
for (let i = 0; i < value.length; i++) {
result[formatName(key, i)] = generateId();
}
}

return result;
},
{
[prefix ?? '']: generateId(),
},
);
return result;
}, {});
}

function setFieldsValidated<Error>(
Expand Down Expand Up @@ -440,8 +438,10 @@ function updateValue<Error>(
if (name === '') {
meta.initialValue = value as Record<string, unknown>;
meta.value = value as Record<string, unknown>;
meta.key = getDefaultKey(value as Record<string, unknown>);

meta.key = {
...getDefaultKey(value as Record<string, unknown>),
'': generateId(),
};
return;
}

Expand Down
3 changes: 2 additions & 1 deletion packages/conform-react/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { FormMetadata, FieldMetadata, Metadata, Pretty } from './context';

type FormControlProps = {
key?: string;
key: string | undefined;
id: string;
name: string;
form: string;
Expand Down Expand Up @@ -214,6 +214,7 @@ export function getFormControlProps<Schema>(
options?: FormControlOptions,
): FormControlProps {
return simplify({
key: metadata.key,
required: metadata.required || undefined,
...getFieldsetProps(metadata, options),
});
Expand Down
73 changes: 1 addition & 72 deletions packages/conform-react/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,82 +91,11 @@ export function useForm<
context.onUpdate({ ...formConfig, formId });
});

const subjectRef = useSubjectRef({
key: {
// Subscribe to all key changes so it will re-render and
// update the field value as soon as the DOM is updated
prefix: [''],
},
});
const subjectRef = useSubjectRef();
const stateSnapshot = useFormState(context, subjectRef);
const noValidate = useNoValidate(options.defaultNoValidate);
const form = getFormMetadata(context, subjectRef, stateSnapshot, noValidate);

useEffect(() => {
const formElement = document.forms.namedItem(formId);

if (!formElement) {
return;
}

const getAll = (value: unknown) => {
if (typeof value === 'string') {
return [value];
}

if (
Array.isArray(value) &&
value.every((item) => typeof item === 'string')
) {
return value;
}

return undefined;
};
const get = (value: unknown) => getAll(value)?.[0];

for (const element of formElement.elements) {
if (
element instanceof HTMLInputElement ||
element instanceof HTMLTextAreaElement ||
element instanceof HTMLSelectElement
) {
const prev = element.dataset.conform;
const next = stateSnapshot.key[element.name];
const defaultValue = stateSnapshot.initialValue[element.name];

if (
prev === 'managed' ||
element.type === 'submit' ||
element.type === 'reset' ||
element.type === 'button'
) {
// Skip buttons and fields managed by useInputControl()
continue;
}

if (typeof prev === 'undefined' || prev !== next) {
element.dataset.conform = next;

if ('options' in element) {
const value = getAll(defaultValue) ?? [];

for (const option of element.options) {
option.selected = value.includes(option.value);
}
} else if (
'checked' in element &&
(element.type === 'checkbox' || element.type === 'radio')
) {
element.checked = get(defaultValue) === element.value;
} else {
element.value = get(defaultValue) ?? '';
}
}
}
}
}, [formId, stateSnapshot]);

return [form, form.getFieldset()];
}

Expand Down
24 changes: 21 additions & 3 deletions packages/conform-react/integrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export function createDummySelect(

select.name = name;
select.multiple = true;
select.dataset.conform = 'managed';
select.dataset.conform = 'true';

// To make sure the input is hidden but still focusable
select.setAttribute('aria-hidden', 'true');
Expand All @@ -98,7 +98,7 @@ export function createDummySelect(
export function isDummySelect(
element: HTMLElement,
): element is HTMLSelectElement {
return element.dataset.conform === 'managed';
return element.dataset.conform === 'true';
}

export function updateFieldValue(
Expand Down Expand Up @@ -306,8 +306,26 @@ export function useControl<
change(value);
};

const refCallback: RefCallback<
HTMLInputElement | HTMLSelectElement | HTMLTextAreaElement | undefined
> = (element) => {
register(element);

if (!element) {
return;
}

const prevKey = element.dataset.conform;
const nextKey = `${meta.key ?? ''}`;

if (prevKey !== nextKey) {
element.dataset.conform = nextKey;
updateFieldValue(element, value ?? '');
}
};

return {
register,
register: refCallback,
value,
change: handleChange,
focus,
Expand Down
6 changes: 0 additions & 6 deletions playground/app/routes/form-control.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,6 @@ export default function FormControl() {
>
Reset form
</button>
<input
type="submit"
className="rounded-md border p-2 hover:border-black"
name="example"
value="Submit"
/>
</div>
</Playground>
</Form>
Expand Down
4 changes: 0 additions & 4 deletions tests/integrations/form-control.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,13 @@ 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")'),
inputButton: form.locator('input[type="submit"]'),
};
}

async function runValidationScenario(page: Page) {
const playground = getPlayground(page);
const fieldset = getFieldset(playground.container);

// Conform should not overwrite the value of any input buttons
await expect(fieldset.inputButton).toHaveValue('Submit');

await expect(playground.error).toHaveText(['', '', '']);

await fieldset.validateMessage.click();
Expand Down
Loading