Component Optimization: Pattern overrides hook

[cbravobernal]

What problem does this address?

Right now, the code inside ControlsWithStoreSubscription function is executed on each block change (including writing a post, each keystroke or hovering a block) regardless the Override block is present or not (it appears only in patterns as far as I know)

gutenberg/packages/editor/src/hooks/pattern-overrides.js

Line 95 in e39ae77

Screen.Recording.2024-07-04.at.14.31.36.mov

How to test

• Add a console log here:

  gutenberg/packages/editor/src/hooks/pattern-overrides.js

  Line 95 in e39ae77
• Write a post with the browser console opened, check that the log appears tons of times.

ping @Mamaduka

[Mamaduka]

Generally, I think it's okay if a component rerenders like this unless it's costly to render and blocks the UI, which doesn't seem to be the case here.

As @ellatrix suggested in the Slack thread, we can optimize this using React.memo and only pass the required props. The props needed for the Control and its child components are clientId, name, metadata and setAttributes.

One issue with memoization is that it's easy to break. Let's say in the future someone updates the code and passes the whole attributes object as props. It will set us back to where we started with + a small cost of component memoization. But we'll have the same issue in the block-editor package, so maybe that's okay.

I created a draft PR just in case - #63189.

@kevin940726, @talldan, what do you think?

[kevin940726]

Are the re-renders costly here? Are we fixing a performance bottleneck or doing premature optimizations? Could this be solved if we leverage the React Compiler in the future?

we can optimize this using React.memo and only pass the required props.

As pointed out, memoing also comes with a cost. We can audit it to see the trade-offs and whether it's worth it.

I'm not opposed to improving it in any way, but I think we should be careful of optimizing based on our assumptions without data support. I appreciate the draft PR explorations a lot! Thanks! ❤️

[cbravobernal]

The component has not been moved. Re-renders are not costly so we don't need any development here.
Thanks @Mamaduka for your contribution!