fabian-hiller / modular-forms

The modular and type-safe form library for SolidJS, Qwik and Preact
https://modularforms.dev
MIT License
1.05k stars 56 forks source link

Qwik: form loader not loading, unexpected <Field/> type error #263

Open genox opened 2 weeks ago

genox commented 2 weeks ago

Hi,

I have this route:

import { component$ } from '@builder.io/qwik';
import type { DocumentHead } from '@builder.io/qwik-city';
import { routeLoader$ } from '@builder.io/qwik-city';
import { Button, Card, Heading, Input } from 'flowbite-qwik';
import { _ } from 'compiled-i18n';
import { useProfileLoader } from '~/lib/route-loader/data/use-profile-loader';
import type { InitialValues } from '@modular-forms/qwik';
import { useForm, valiForm$ } from '@modular-forms/qwik';
import * as v from 'valibot';

export { useProfileLoader };

const ProfileSchema = v.object({
  firstname: v.pipe(v.string(), v.nonEmpty('Please enter your firstname.')),
  name: v.pipe(v.string(), v.nonEmpty('Please enter your family name.')),
});

type ProfileForm = v.InferInput<typeof ProfileSchema>;

export const useFormLoader = routeLoader$<InitialValues<ProfileForm>>(async (requestEvent) => {
  const profile = await requestEvent.resolveValue(useProfileLoader);
  if (!profile) {
    return {
      firstname: 'test',
      name: 'test',
    };
  }
  console.log(profile);
  const { firstname, name } = profile;
  return {
    firstname,
    name,
  };
});

export default component$(() => {
  const [profileForm, { Form, Field }] = useForm<ProfileForm>({
    loader: useFormLoader(),
    validate: valiForm$(ProfileSchema),
  });

  return (
    <Card>
      <div class={'flex flex-col gap-8'}>
        <Heading tag={'h2'}>{_`Your Profile`}</Heading>
        <Form>
          <Field name="firstname">{(field, props) => <Input {...props} type="email" />}</Field>
          <Field name="name">{(field, props) => <Input {...props} type="text" />}</Field>
          <Button type="submit">{_`Save`}</Button>
        </Form>
      </div>
    </Card>
  );
});

Issues

  1. The form is not being populated with initial values, even though the routeLoader correctly logs the data on the server side
  2. complains about a missing of= attribute that should be the formStore (profileForm), but even when supplying that it complains about mismatched types. Has no effect on missing initial values.

I mainly followed the tutorial on your website since I was away for a while and just returned to working with Qwik and didn't want to run into breaking changes I was not aware of.

I can't find anything that makes this differ much from the example, yet it doesn't work and there are weird type issues that should not (no?) be there. can you see something obvious?

Thanks for having a look

Versions:

node 22, ts 5.6.2

genox commented 2 weeks ago

I replaced the custom Input with regular input but there was no change in behaviour

genox commented 2 weeks ago

It looks like value= needs to be set manually, spreading props is not enough. At some point I talked to one of the qwik devs and he suggested that prop spreading might not be such a great idea, but honestly I don't remember the context .. but it had to do with modular forms.

Anyways, when I set value={field.value} the form is correctly populated.

Not sure if this needs to be reflected in the docs: https://modularforms.dev/qwik/guides/add-fields-to-form

edit. as a heads-up, Flowbite-Qwk does not play nice with modular forms, for the same reason: props spreading. I have no idea why I seem to be the only one affected by that, but prop spreading caused a lot of issues for me in the past and now again.. 🤦

fabian-hiller commented 2 weeks ago

Is this the problem? https://modularforms.dev/qwik/guides/controlled-fields

genox commented 2 weeks ago

Well, it might be.

The example I mentioned does use initial values and seems like a "more complex form" and has no value={field.value}. See bottom of the page: https://modularforms.dev/qwik/guides/validate-your-fields

Is it good to have 2 way of doing things when one works always and the other might not work based on rules that are only later established in the documentation and are not really transparent (technically, I mean)?

I think it would be a better DX to have field.value separately attached in all cases, established as the default, then the issue doesn't even come up.

fabian-hiller commented 2 weeks ago

The documentation teaches the library piece by piece, and controlled fields are not necessary in basic cases. That's why the first guides are kept simple and don't include this code. But I will consider your feedback in the long run. Maybe we should change this.

genox commented 2 weeks ago

I think the documentation is great, it gets you started very quickly and you did a great job to introduce one thing at a time. Don't get me wrong. But this thing .. I am sure I stumbled over this for the second time now.

As a concrete suggestion I would like to add, that I think it would greatly help to mention this in some sort of infobox as soon as Fields are mentioned with an additional concise description of what makes a form qualify as "complex" and maybe even a short explanation of the underlying technical reasons.

Something like "A complex form is one that uses useFormLoader, implements custom components instead of directly using primitive HTML form elements or ..."

Then it is clear right from the beginning. Right now it is kind of a foot note and over ambitious devs might not even see it. I guess I qualify for this, or maybe its the attention span .. ;)

But yeah, just my experience. You are doing a great job with this library, it's great to use once the pitfalls are overcome and it makes life a lot easier.