edmundhung / conform

A type-safe form validation library utilizing web fundamentals to progressively enhance HTML Forms with full support for server frameworks like Remix and Next.js.
https://conform.guide
MIT License
1.77k stars 99 forks source link

getFieldset doesn't correctly proxy a nested class object #593

Open ngbrown opened 4 months ago

ngbrown commented 4 months ago

Describe the bug and the expected behavior

I have a form with initial values (defaultValue) and a nested array, but the initialValue was always undefined. It took me awhile to figure out what is going on, but it appears that it was because the nested object was a class. This is a simple class with just public properties.

What I expect to work:

import { useForm } from '@conform-to/react';

class Todo {
  id: number;
  title: string;

  constructor(id: number, title: string) {
    this.id = id;
    this.title = title;
  }
}

const formDefault = {
  todos: [
    new Todo(3, 'do something'),
    new Todo(4, 'do another thing'),
  ],
};

export default function Index() {
  const [form, fields] = useForm({
    defaultValue: formDefault,
  });
  const todos = fields.todos.getFieldList();

  return (
    <div>
      <form id={form.id}>
        <ul>
          {todos.map((todo) => {
            const todoFields = todo.getFieldset();
            return (
              <li key={todo.key}>
                <input
                  hidden
                  readOnly
                  name={todoFields.id.name}
                  value={todoFields.id.initialValue}
                />
                <input
                  name={todoFields.title.name}
                  defaultValue={todoFields.title.initialValue}
                />
                <div>{todoFields.title.errors}</div>
              </li>
            );
          })}
        </ul>
      </form>
    </div>
  );
}

Conform version

v1.1.0

Steps to Reproduce the Bug or Issue

View this snippet: https://stackblitz.com/edit/remix-run-remix-cym13r?file=app%2Froutes%2F_index.tsx

In the stackblitz snippet, you can alternate between using a class and plain JavaScript objects. The location to access the id value changes between the two. It should be consistent.

What browsers are you seeing the problem on?

Chrome, Firefox

Screenshots or Videos

No response

Additional context

The actual class of values is generated for protobufjs, but I reproduced the problem with just a simple class and constructor.

The inability to proxy a class also seems to be applicable at the top level. See the ~/routes/edit.$id._index/route.tsx file in the above snippet to view that.

edmundhung commented 4 months ago

I don't think we can support a class instance in the default value unless it is serializable (This is not implemented, just an idea). Conform does an additional normalization step to ensure that the default value can be compared with the form value for additional checks such as the dirty state. But this won't work correctly in your case.

You will need to convert it to an object literal before passing it to the default value. Conform should probably throw an error if it finds this as well.

lifeiscontent commented 4 months ago

@edmundhung it might make sense to loosen up the API given the following example:

Playground Link

its not totally illogical to expect this to work

given the reality that we won't be able to serialize back to the class