rescriptbr / reform

📋 Reasonably making forms sound good
https://rescript-reform.netlify.app/
MIT License
354 stars 41 forks source link

Field doesn't rerender on render prop change #155

Closed a-gierczak closed 3 years ago

a-gierczak commented 4 years ago

Because of this useMemo Field component won't get rerendered when renderProp changes. I know this is for optimization purposes, but I think we should be able to opt-out from it.

Consider this scenario:

let make = () => {
  let (isDisabled, setIsDisabled) = React.useState(_ => false);

  /* some external event  */
  React.useEffect1(() => {
    setIsDisabled(_ => false);
  }, ...);

  <Field field render={({value, handleChange}) => {
    <input value onChange=handleChange disabled=isDisabled />
  }}
};

When isDisabled changes input won't be enabled, because of stale closure.

fakenickels commented 4 years ago

Thanks for reporting.

When I need to do a special rendering for a input I always end up removing it out of the Field component. But if it'll help ergonomics maybe we can have a dependencies optional param for the Field component.

<Field dependencies=[|isDisabled|] />

I don't wanted to extend too much the functionality of Field, but we can analyse the situation

a-gierczak commented 4 years ago

I think better approach would be to not memoize by default, because bugs like that may be confusing for new users and in most cases, additional renders don't cause that much performance overhead. Also using Field may be the default approach for new users, providing that it's documented in "basic usage" on this repo :)

Same functionality could be achieved by using useCallback on the render prop.

let render = React.useCallback0(({value, handleChange}) => {
  <input value onChange=handleChange />
});

<Field field render />
jazithedev commented 3 years ago

Hello! Have similar issue. Initially I start with disabled <select> field with no options, and after clicking a separate button (which loads data), the options and disabled attributes should change. It is not happening because the component inside of render prop is not rerendered. Can something be done with this?

jazithedev commented 3 years ago

@fakenickels

Any info if this issue is resolved? The PR is still "Open" :).

fakenickels commented 3 years ago

I'm doing a clean up getting ready for the upgrade and we'll keep up with this issue soon 😉