-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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 optimizations #54
Comments
Thank you for this report 👍 |
If we want to properly implement But I agree the impact on performances on lower end machines might get really important, so we should do this upgrade. (And yeah, kudos for the super clean and useful report.) |
Again, pardon my ignorance of the codebase and whether these suggestions have already been considered, but it's quite possible to perform shallow comparison (i.e. pure render) even with plain object properties, so long as you're careful not to mutate the original when updating. Object.assign( {}, original, { input: 'New Value' } ); Or, for deeply nested objects, consider pulling in merge( {}, original, { foo: { bar: { input: 'New Value' } } } ); |
The problem is not with copying/merging deeply nested objects, it's with comparing them to know if they've changed. ({a: 1, b: {c: 2}}) === ({a: 1, b: {c: 2}}) // false And Am I missing something obvious? I didn't have my first coffee of the day just yet. |
Does this affect the demo only or the form rendering itself? |
@leplatrem : I'm observing it in my own usage as well, but the demo was the easiest way for me to illustrate the problem. |
It may be related to kinto-admin, but when viewing a page with a few hundred records (for the blocklist), the page is very laggy, unresponsive, and even freezes for a couple of seconds at the beginning. |
Might be compounded with the fact that JSON schema validation is performed on every change: |
This is probably the very first thing to investigate as a solution https://facebook.github.io/react/docs/shallow-compare.html |
I have used a custom debounce of 2 seconds in onChange to improve type speed as a temporary solution |
Loving this component so far! However, in my own usage, and even in the demo site, there's a noticeable lag when attempting to enter text into fields at a reasonable typing speed. Enabling the "trace react updates" option in Chrome React Developer Tools, it becomes apparent that the entire form is re-rendered any time an input changes.
I haven't dug into the code, but it would seem reasonable to implement
shouldComponentUpdate
(perhaps simply withpure-render-mixin
) to prevent rerendering of components not affected by the change in form values.The text was updated successfully, but these errors were encountered: