Giter Club home page Giter Club logo

Comments (8)

aliemir avatar aliemir commented on June 10, 2024 1

Thank you for the investigation @misterx! #5831 #5851 will prevent the consumption of signal and restore the older behavior for your case then. About the useEffect calling resetFields(), I think it makes sense, if you want to open up a PR for this change, we'll be happy to include it in our next release 🚀

Linked the wrong PR 🤦

from refine.

misterx avatar misterx commented on June 10, 2024

I found why the duplication happens. I copied the useList hook into my local project for debugging and discovered that accessing the signal from queryFn triggers refetching

queryFn: ({ queryKey, pageParam, signal }) => {

When I removed the signal from queryFn, everything worked well. I also found a related ticket about this issue: TanStack/query#6089.

I'm not sure how I can fix this from my project.

from refine.

aliemir avatar aliemir commented on June 10, 2024

Hey @misterx, sorry for the issue! I've tested in my local using the code you've provided but haven't see anything unusual 🤔 I've observed that the ResourceSelect ends up sending a duplicate request due to strict mode. When I disable strict mode or test it in production I can see that there's no duplicate request. Do you have a duplicate request in production or when you disable strict mode?

About the signal issue you've referenced. I think we can fix this, we can refactor those lines to not use destructuring and make access of signal property with a getter to avoid unexpected side effects from @tanstack/react-query. I think doing so will also prevent duplicate requests (as you've experimented in your local) in strict mode.

from refine.

misterx avatar misterx commented on June 10, 2024

My investigation has revealed the following issues:

Duplicate Requests:
Each input component is mounted three times, causing three duplicate requests:

Component Remounting:
According to the antd documentation, using resetFields leads to the remounting of input components (see more at https://ant.design/components/form#why-will-resetfields-re-mount-component).

Caching and Query Cancelation:
Every new render triggers a new request because the useList hook consumes the signal property in queryFn.
The useQuery documentation (https://tanstack.com/query/v4/docs/framework/react/guides/query-cancellation#default-behavior) states that caching will occur on remounts if the signal property is not consumed. If it is, then the responsibility for cancelling the request falls to the client code.

Behavior Control in Older Versions:
In earlier versions of Refine where the signal property is not utilized, client-side behavior can be managed using the staleTime property in queryOptions.

@aliemir I've created sandbox https://codesandbox.io/p/devbox/pedantic-albattani-cymd3r where I removed strict mode and see three identical requests.

image

I also created a simple component that shows the input remounting.

export const TestInput: React.FC = () => {
  const ref = useRef(0);
  ref.current++;
  console.log("Current ref");
  console.log(ref.current);
  return <></>;
};

Result in the console (you can see the same in the sandbox):
image

As you can see, the component is created three times.

from refine.

misterx avatar misterx commented on June 10, 2024

Also, one unnecessary render can be removed if resetFields() is called when queryResult?.data?.data is not undefined.

https://github.com/refinedev/refine/blob/master/packages/antd/src/hooks/form/useForm.ts#L262-L264

from refine.

misterx avatar misterx commented on June 10, 2024

@aliemir I would be glad to contribute to the project, but I'm not sure I have enough knowledge to commit since I'm a backend developer and not familiar with frontend development tools such as testing and building tools. After my research, I understand why Refine uses form.resetFields(): it's necessary to set the initial values from the request. I've researched how others address this issue to avoid re-rendering and found a solution from AntProForm

https://github.com/ant-design/pro-components/blob/9daed82b9a1617131cfd22393969131e1f45568c/packages/form/src/BaseForm/BaseForm.tsx#L720-L729

they also load initial values from a request but do not render the form until the values are ready. Currently, I can't use this approach with the existing Refine useForm because the form.resetFields call from refine.useForm is not customizable and can't be disabled.

React.useEffect(() => {
form.resetFields();
}, [queryResult?.data?.data, id]);

So, I've created a simple hack (I'm not sure this is a good solution, but it works) that prevents unnecessary form refreshes on the Edit form until the data is defined.

const useFixFormResetOnLoad:(form:FormInstance,initialValues?:any)=>void = (form,initialValues) => {
    const originalReset = useRef<FormInstance['resetFields']>(form.resetFields);
    useEffect(()=>{
       form.resetFields = initialValues===undefined?()=>undefined:originalReset.current;
    },[initialValues, form])
}

And edit page will be something like:

export const BlogPostEdit = () => {

  const {form,formProps,saveButtonProps, queryResult, formLoading } = useForm();
  useFixFormResetOnLoad(form,queryResult?.data?.data);
  if(formLoading){
    return (
        <div style={{ paddingTop: 50, paddingBottom: 50, textAlign: 'center' }}>
	    <Spin />
        </div>
    );
  }

   return (
    <Edit saveButtonProps={saveButtonProps}>
        <Form layout="vertical" {...formProps}>
        {/*.............................*/}
        {/*.... standard refine code ...*/}
        {/*.............................*/}
        </Form>
    </Edit>
}

This behavior is good for me because it avoids unnecessary re-rendering for complex forms. However, I'm not sure if this approach will be suitable for others, as form is not visible durring initialData loading.

from refine.

aliemir avatar aliemir commented on June 10, 2024

@misterx thank you for taking your time for the investigation and explanation. Understood the issue here and your workaround. If I understood correctly, we can try to add a simple condition here at

React.useEffect(() => {
form.resetFields();
}, [queryResult?.data?.data, id]);

and then the initial rendering can be handled by users by conditionally rendering via formLoading, am I correct?

We may also do some hacks in formProps (like the one you did in useFixFormResetOnLoad) to trick initial rendering but this will change the current behavior and might be unwanted in some cases.

In conclusion, we'll make form.resetFields to be called conditionally in the effect and leave the handling of the initial render to the user. 🙌 Until this is done and released, your workaround will work the way we expect 🚀

from refine.

misterx avatar misterx commented on June 10, 2024

@aliemir Yes, you are correct. Also, since the first and second renders are the same (the state is unchanged and the initialValues are empty), form.resetFields can be called only once when the data is received. This will eliminate the unnecessary second re-render and should not impact existing client code.
Furthermore, removing re-rendering will enhance the framework's functionality, allowing for simpler use of Form.List with useSelect. An example use case is creating an invoice with lines where the product on each line should be a dropdown from the products resource (this is similar to the use case that prompted this investigation). I see the refine-crm example with a similar form on the QuotesShow page https://example.crm.refine.dev/quotes/show/84, but without dropdowns ;-)

from refine.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.