Canvas Performance: ComponentRendererComponent shouldUpdate() problems #6066
Assignees
Labels
Comments
balazsbajorics
changed the title
liady
pushed a commit
that referenced
this issue
Dec 13, 2024
**Problem:** See #6066 **Fix:** The major contributor to the slowness of `isElementInChildrenOrPropsTree ` is `React.children.toArray`. I tried to use `React.children.forEach` or `React.children.map` but those just made things worse. However, we know that in lot of cases `props.children` is just an array of elements or a React element itself. In these cases there is no need to run the fully general and slow toArray. Even though this is not the official use of the API (it is actually discouraged: https://react.dev/reference/react/Children#why-is-the-children-prop-not-always-an-array), it makes sense to try to get the elements without calling toArray (and we can still call `toArray` when that fails). I made the function even more faster by switching to simple `for` loops instead of using array functions and allocating even more arrays. With these changes I never saw >1.3ms shouldUpdate times when resizing a PromiseCard. **Manual Tests:** I hereby swear that: - [x] I opened a hydrogen project and it loaded - [x] I could navigate to various routes in Preview mode Fixes #6066
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When resizing a PromiseCard component on the canvas,
we spend 6ms on
shouldUpdateinside the componentRendererComponent. Bottom up, it spends most of that time inisElementInChildrenOrPropsTree->React.Children.toArray.This should be blazing fast, because this is called in a very tight loop.
The text was updated successfully, but these errors were encountered: