refinedev / refine

A React Framework for building internal tools, admin panels, dashboards & B2B apps with unmatched flexibility.
https://refine.dev
MIT License
28.38k stars 2.2k forks source link

[BUG] @refinedev/react-hook-form useForm not detecting changes correctly #6306

Open pcfreak30 opened 2 months ago

pcfreak30 commented 2 months ago

Describe the bug

I have been debugging the functionality for warnWhenUnsavedChanges and I found that react-hook-form only ever emits a type with subscription event objects on blur?

At https://github.com/refinedev/refine/blob/master/packages/react-hook-form/src/useForm/index.ts#L223-L230 your checking the type, which causes value changes to never trigger the onValuesChange callback and so the unsaved changed warn never works.

Steps To Reproduce

  1. Create a shadcn based form with react-hook-form and warnWhenUnsavedChanges enabled. This should be inside a dialog.
  2. Add a breakpoint on that code.
  3. Find that it never triggers b/c the state never includes a type when value is passed.

Expected behavior

warnWhenUnsavedChanges should function correctly with state change detection enabling it.

Packages

Additional Context

No response

BatuhanW commented 2 months ago

Hey @pcfreak30, could you share your form code with us? We could better understand with a reproducible example. We aren't quite sure how you handle inputs, so we need to check.

pcfreak30 commented 2 months ago

@BatuhanW this is actively wip, so im using a specific commit hash, but you can see at https://github.com/LumeWeb/web/blob/5a41d85d272e977790b4d90623aa9b4a6d08f0a2/apps/portal-dashboard/app/routes/service/components/PinDialog.tsx

i have a ugly workaround atm via:

    const subscription = form.watch((value, { name, type }) => {
      if (value && !type) {
        form.control._subjects.values.next({
          value,
          name,
          type: "change",
        });
      }
    });
    return () => subscription.unsubscribe();
  }, [form.watch]);

I had AI try to create a minimal example, but I haven't tested it:

import React from 'react';
import { useForm } from 'react-hook-form';
import { useModalForm } from '@refinedev/react-hook-form';
import { Button } from "@/components/ui/button";
import {
  Dialog,
  DialogContent,
  DialogDescription,
  DialogFooter,
  DialogHeader,
  DialogTitle,
  DialogTrigger,
} from "@/components/ui/dialog";
import { Input } from "@/components/ui/input";
import { Label } from "@/components/ui/label";

export const ShadcnDialogWarnUnsavedChangesBug: React.FC = () => {
  const form = useModalForm({
    refineCoreProps: {
      action: "create",
      resource: "example",
    },
    warnWhenUnsavedChanges: true,
  });

  const { register, formState: { isDirty } } = form;

  return (
    <Dialog>
      <DialogTrigger asChild>
        <Button variant="outline">Open Dialog</Button>
      </DialogTrigger>
      <DialogContent className="sm:max-w-[425px]">
        <DialogHeader>
          <DialogTitle>Edit profile</DialogTitle>
          <DialogDescription>
            Make changes to your profile here. Click save when you're done.
          </DialogDescription>
        </DialogHeader>
        <form>
          <div className="grid gap-4 py-4">
            <div className="grid grid-cols-4 items-center gap-4">
              <Label htmlFor="name" className="text-right">
                Name
              </Label>
              <Input
                id="name"
                className="col-span-3"
                {...register('name')}
              />
            </div>
          </div>
          <DialogFooter>
            <Button type="submit">Save changes</Button>
          </DialogFooter>
        </form>
        <div>Form is dirty: {isDirty ? 'Yes' : 'No'}</div>
      </DialogContent>
    </Dialog>
  );
};
aliemir commented 1 month ago

Hey @pcfreak30 👋, I've checked the code you've provided and investigated the issue for a bit. Looks like calling setValue doesn't add type to the subscription calls but registered controls/fields do. Since this side is done by react-hook-form, altering the internal functionality may break other projects and introduce an additional complexity 🤔 In this case you should use the field.onChange to apply changes instead of setValue. Let me know if this works out for you 🙏

pcfreak30 commented 1 month ago

Hey @pcfreak30 👋, I've checked the code you've provided and investigated the issue for a bit. Looks like calling setValue doesn't add type to the subscription calls but registered controls/fields do. Since this side is done by react-hook-form, altering the internal functionality may break other projects and introduce an additional complexity 🤔 In this case you should use the field.onChange to apply changes instead of setValue. Let me know if this works out for you 🙏

Um... could you please provide an example so im clear?