Skip to content

Commit

Permalink
fix: revert auto field value update (#778)
Browse files Browse the repository at this point in the history
  • Loading branch information
edmundhung authored Sep 19, 2024
1 parent 24105f4 commit 01130bd
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 104 deletions.
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

0 comments on commit 01130bd

Please sign in to comment.