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

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

Closed
pcfreak30 opened this issue Sep 6, 2024 · 6 comments
Closed
Labels
bug Something isn't working

Comments

@pcfreak30
Copy link

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

@pcfreak30 pcfreak30 added the bug Something isn't working label Sep 6, 2024
@BatuhanW
Copy link
Member

BatuhanW commented Sep 6, 2024

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
Copy link
Author

@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
Copy link
Member

aliemir commented Sep 16, 2024

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
Copy link
Author

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?

Copy link

stale bot commented Nov 15, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Nov 15, 2024
@stale stale bot closed this as completed Nov 22, 2024
@aliemir aliemir reopened this Nov 25, 2024
@stale stale bot removed the wontfix This will not be worked on label Nov 25, 2024
@aliemir
Copy link
Member

aliemir commented Nov 25, 2024

Sorry for the late response. Instead of having setValue to update the form values according to the inputs, you should use the onChange from the register("my-field")'s return values or onChange from the render function of the <Controller> component.

with register

const { onChange, onBlur, name, ref } = register('firstName'); 

onChange("something");

with <Controller />

<Controller
        control={control}
        name="firstName"
        render={({ field: { onChange, onBlur, value, ref } }) => (
          /* use `onChange` to set the value */
        )}
/>

instead of

setValue("firstName", "something");

@aliemir aliemir closed this as completed Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants