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

Performance issues in v2 #1974

Closed
mvestergaard opened this issue Oct 30, 2019 · 1 comment · Fixed by #2070
Closed

Performance issues in v2 #1974

mvestergaard opened this issue Oct 30, 2019 · 1 comment · Fixed by #2070

Comments

@mvestergaard
Copy link
Contributor

🐛 Bug report

Current Behavior

Changes to any field (both Field and FastField) causes all fields in the form to re-render twice.

Expected behavior

Changes to fields should only cause fields to re-render once, and more over, FastField should only re-render when changes are made to that field specifically.

Reproducible example

Here's the same scenario demonstrated in both v2 and v1
v2.0.3 https://codesandbox.io/s/vigorous-wood-esy70
v1.5.8 https://codesandbox.io/s/keen-bouman-sx5r0

Suggested solution(s)

Additional context

According to the react dev tools, the extra renders happen because "Context changed".

Possibly same problem as reported on #1759

Your environment

Software Version(s)
Formik 2.0.3
React 16.8.4
TypeScript 3.6.2
Browser Chrome
npm/Yarn yarn
Operating System windows
@punmechanic
Copy link

punmechanic commented Oct 31, 2019

Yes, looking into this I'm seeing that onChange from useField() is changing twice every single time a value in the form changes.

This can be seen in this sandbox - Note that the counter displayed increases twice every time you enter a character in the text box. That counter is only incremented when onChange changes.

One way to fix this temporarily until a better fix lands in master/is suggested by Jared would be to only use the initial version of onChange from useField(). You can do this using a Ref:

function Input({ name }) {
  const [fieldProps] = useField(name)
  const onChange = React.useRef(fieldProps.onChange)
  return <input ... onChange={onChange.current} />
}

Which will prevent child components from re-rendering if only onChange changes from the formik context.

This is definitely a bad idea (it assumes implementation detail) and should be removed as soon as the perf issues are resolved within the library itself. From reading the code around handleChange/executeChange, this will only break if you're using a checkbox - The executeChange code relies upon using state.values to update a checkbox state, which, using this code, will be stale inside the onChange closure.

(Wow, I love hooks!)

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

Successfully merging a pull request may close this issue.

2 participants