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

Form always dirty? #577

Open
JasonColeyNZ opened this issue Apr 11, 2024 · 11 comments
Open

Form always dirty? #577

JasonColeyNZ opened this issue Apr 11, 2024 · 11 comments

Comments

@JasonColeyNZ
Copy link

Describe the bug and the expected behavior

After installing 1.1.0 it appears that form.dirty is always true. I have Save Buttons disabled unless there have been changes, these are always enabled now, they are controlled disabled={!form.dirty}

Conform version

c1.1.0

Steps to Reproduce the Bug or Issue

disabled={!form.dirty}

What browsers are you seeing the problem on?

No response

Screenshots or Videos

No response

Additional context

No response

@edmundhung
Copy link
Owner

Any chance you were rendering an input conditionally with a default value? Conform derives the dirty state simply by comparing the default value you provided with the latest form value. With v1.1, it also watch the doms on what is actually rendered to keep the form value in sync with the form. So if you got any input with a default value not rendered, it will consider dirty.

@JasonColeyNZ
Copy link
Author

I have hidden inputs, but surely they are ok? All controls used are Radix/UI, I will test removing these one at a time until I find if there any any custom control implementations that could be causing this

@JasonColeyNZ
Copy link
Author

OK that was it, I didn't include the 'intent' fields from my schema in the defaultValues. OK looks good now, just a few more rules to get this working 100%, however completely makes sense.

@houmark
Copy link

houmark commented Apr 13, 2024

This also lifted the dirty state on my form, and the solution does not seem that simple in my case.

I think this change may create a lot of issues for this library in terms of being too strict and not allowing some additional fields that are not handled by Conform and should not be.

There can be many reasons for having such fields and reasons for not wanting Conform to manage them or use them for its dirty state check or anything else for that matter.

I tried an approach using the unstable useControl utility to handle some of the issues I had, but that feature is barely documented and it's not super clear to me how it works even though I read the PR comments.

Maybe a helper utility that can be added as a prop on any (hidden) field which tells Conform to exclude this field from the comparison, etc., could be useful here? It also seems to me that including fields in defaultValue in the useForm hook that does not have a field can affect this comparison? In my case, I have some date/time fields that I am loading from the DB which are displayed in the UI but not updated and thus they don't have form fields. Basically I am using my loaded record from the DB as default values (which I think is okay, no?) but now it seems I need to filter the fields out I don't have form fields for before adding them to defaultValue in order to please Conform.

(A bit later).... So after having wrote all this out and pondering a bit more without sending it, it seems using defaultValue in a more explicit way to define every exact field in the form manually rather than including the entire "edit record" more or less solves the issue. Maybe a helper function here is a better approach where you can pass in a loaded record that will then keep only fields that are in the schema and set default values based on that. I could probably make my own here, but wanted to pass on thoughts from someone just trying to utilize Conform, and maybe some of it gives you inspiration for improvements or ways to keep everything strict while not putting too much additional workload on the developer.

@JasonColeyNZ
Copy link
Author

@houmark I also had the same issue where I had in my objects dates and other info that I wanted for other controls on the form. And, like you had to remove these from my object before passing into Conform, this was a pain for sure.

I like the idea that the schema should be the source of truth as to which fields are checked as part of the defaultValue. As this is what we are checking against for form validation.

What do you think @edmundhung ?

@houmark
Copy link

houmark commented Apr 13, 2024

Yeah, I cannot think of any downside with Conform parsing the defaultValue using the schema from constraint if it's passed to the useForm hook (since constraint it's not required) and discarding any key that is not in the constraint. That, or just ignoring fields that are not in the form but in the defaultValue so it's more forgiving.

It's hardish to debug or even understand why the form became dirty. I had to debug out form.value to understand what was happening, but even that was not super obvious, as it would be quickly re-render the values and kinda hide some of the info that was useful. So with that in mind, maybe Conform should debug out common issues like this in the console )or alternatively include some helper that shows this information rendered) if in dev mode. Conform does a lot of "magic" and "behind the scenes" stuff and I think some people will either drop using Conform, run incorrectly (form always dirty) or have some frustration getting things working correctly if Conform is as strict as now without automatically handling some of this or providing better debug information.

Anyways, just my two cents — and I have now gotten my form to play 99% good with Conform and clean/dirty state.

For what it's worth, here's my debugging code for checking how Conform sees in the form, it should match the form fields you have (including any hidden fields) — and keep an eye on when it just loads the form as it can quickly shift after the first render(s):

<pre>
  {JSON.stringify(form.value)}
</pre>

@JasonColeyNZ
Copy link
Author

@houmark cheers for the debug code, I might use this in some more complicated forms.

I must admit though that 1.1.0 has fixed my dirty form issues I had with tabbing through fields, and I love that I can make a change in a field, then revert the changes and the form dirty state goes back to false.... exactly how I want it to work! cheers @edmundhung for another great update!

@houmark
Copy link

houmark commented Apr 13, 2024

Yes, and I'm sorry if that wasn't clear in my prior comments. I am a fan of this change in the end, but it was a bit tough to get to the happy place and I can see others struggling with getting there when first using Conform or when updating, especially with the current documentation. That's why I'm suggesting either better internal handling to remove most of the potential blockers/hold ups and/or add automatic console logging or helper functions for debugging, so it becomes easier to visualize what is holding up a happy Conform.

The 1% that has not worked for me in my lightly advanced form is resetting which I cannot get to work fully for all fields (some are using unstable_useControl to handle values to a TipTap editor) no matter how I attempt to debug, but it's a minor thing for me as I don't actually need a form reset option in this particular form, so I will either circle back to it or hope this is a bug that gets fixed later in Conform.

@edmundhung
Copy link
Owner

Yeah, I cannot think of any downside with Conform parsing the defaultValue using the schema from constraint if it's passed to the useForm hook (since constraint it's not required) and discarding any key that is not in the constraint.

This won't work reliable because there is no way to assume that you have a constraint for every field. It also wasn't a required option so it won't work for other cases as well.

That, or just ignoring fields that are not in the form but in the defaultValue so it's more forgiving.

Conform never maintains the form value. It syncs the value with the FormData API so anything get rendered in the form will be reflected on form.value. So, instead of maintaining the form value manually on the client, we are sure that the difference between server and client are minimized. The downside, as you have found, is less forgiving especially when dealing with the dirty state.

There are two solutions right now:

  1. Include the value in initialValue as mentioned before, or
  2. Set the shouldDirtyConsider() option on useForm to ignore some of the fields:
function Component() {
  const [form, fields] = useForm({
    shouldDirtyConsider(name) {
       // e.g. ignore intent and CSRF token that are not mangaged by Conform
       return name !== 'intent' || name !== 'csrf_token';
    },
  });

  // ...
}

@JasonColeyNZ
Copy link
Author

The shouldDirtyConsider is perfect, thanks, I hadn't spotted this.

Is there a way to see which fields are considered dirty?

I have some quite complex forms with many nested objects and arrays, would be good if there was a way to tell which property has caused the for to be dirty. At present it is quite the task to check a complex form for the offending field/control. Especially if I have a component library with custom components.

@JasonColeyNZ
Copy link
Author

JasonColeyNZ commented Apr 24, 2024

Actually is appears that the shouldDirtyConsider doesn't process nested objects, I have a console output and it isn't processing each property of an object, is this correct?

I have also noticed that the shouldDirtyConsider looks at the form controls, but the issue we have is when there are extra fields in the defaultValue... hmm, not an easy one to sort out, especially with complex forms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants