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

Allow forcing synchronous block rendering #17638

Closed
chrisvanpatten opened this issue Sep 29, 2019 · 6 comments
Closed

Allow forcing synchronous block rendering #17638

chrisvanpatten opened this issue Sep 29, 2019 · 6 comments
Labels
[Package] Data /packages/data [Type] Performance Related to performance efforts

Comments

@chrisvanpatten
Copy link
Contributor

chrisvanpatten commented Sep 29, 2019

Is your feature request related to a problem? Please describe.

In #13056, an "async" mode was introduced to the data module which allows a React component tree to be re-rendered asynchronously. The primary use today is in the block list, to prevent all blocks from re-rendering on changes in the stores.

Practically, this means that dispatching an action on a store will cause the re-rendering of unselected blocks to appear delayed/throttled.

This change was implemented primarily to solve some performance related typing issues (#11782), but can be frustrating when building highly visual block UIs which render based on values changing in a store. This is especially obvious when multiple blocks are visible on the screen, and you can clearly see a 'staggered' re-rendering (roughly ~0.5s per block).

Describe the solution you'd like

It is currently possible to override this behavior by wrapping your block in the AsyncModeProvider HOC and setting value={ false }, however it introduces obvious performance issues in that the block is now re-rendering in response to any changes in the store.

Even better would be a way to declare specific actions as "high priority", so the synchronous re-rendering is scoped and limited to cases where non-async re-rendering is truly necessary. This would allow developers to allow "instant" re-rendering while preserving broader performance benefits.

Describe alternatives you've considered

I have considered other approaches such as using block attributes and dispatching updateBlock actions. I believe block attributes always render synchronously, and thus would not be subject to this limitation. However, it's a more complex solution and could cause editor UI settings to be persisted to post_content. Per the comments, block attribute actions are not rendered synchronously either.

@chrisvanpatten chrisvanpatten added [Type] Performance Related to performance efforts [Package] Data /packages/data labels Sep 29, 2019
@youknowriad
Copy link
Contributor

This is already possible, it's still marked experimental. you can wrap your edit function in wp.data.__experimentalAsyncModeProvider with a value prop set to false.

@chrisvanpatten
Copy link
Contributor Author

chrisvanpatten commented Sep 30, 2019

@youknowriad I know, and noted that as my solution under “Describe the solution you'd like” 🙂

However it seems like an even better solution would be a more granulated approach (if such a thing is possible) that allows you to have the best of both worlds: the async mode is used in most cases when performance is a priority, and disabled only in cases where it is absolutely necessary (which a developer would somehow indicate).

@youknowriad
Copy link
Contributor

I believe block attributes always render synchronously, and thus would not be subject to this limitation

that's not true in the description. All changes render asynchronously in unselected blocks right now including block attributes.

I'm not yet understanding what you're looking for :) The performance issues we have are not related to a given change, but to the fact that any change trigger selectors calls to all components that are "synchronously" connected to the state;

@chrisvanpatten
Copy link
Contributor Author

that's not true in the description. All changes render asynchronously in unselected blocks right now including block attributes.

Ah rats, my misunderstanding! I’ll strike that out.

Perhaps an example is better. In one of our content types, we allow editors to toggle a “Compact Mode”. Without getting too much into the details, it’s basically an alternate view of the blocks in the content area.

The enabling/disabling of Compact Mode happens in a custom “settings” store (from a toggle in a sidebar) and then within the block’s edit component we basically check that value (via withSelect) and conditionally display a different component based on the value.

That works, but produced an effect in the UI where you see each block re-render, one after another (with a half second gap between), creating an awkward staggered appearance while you wait for all the blocks to be switched to “compact” view.

This is obviously awkward, and on content with more blocks it could take you 5-10 seconds while the blocks all adjust.

So that’s ultimately what I’m avoiding by “forcing” synchronous re-rendering, but I’m concerned that in the process I’ve also created performance problems down the road.

Or perhaps I shouldn’t be concerned, and maybe I’ve misunderstood how the asynchronous mode works — in which case I’m very happy to close this ticket :)

@youknowriad
Copy link
Contributor

Thanks for clarifying 👍

So that’s ultimately what I’m avoiding by “forcing” synchronous re-rendering, but I’m concerned that in the process I’ve also created performance problems down the road.

You're indeed introducing performance issues for long posts. Last I checked, with more than 200 blocks and sync rendering, the editor lags.

--
That said, I think you can accomplish what you want by using a global state to serve as an "synchronous transaction" where you disable async mode, perform your action, enable async mode. during that interval, the editor becomes a bit laggy, so maybe something you could handle by showing a "loading" or something like that. You can maybe test it at least.

@chrisvanpatten
Copy link
Contributor Author

This can be resolved, especially now that the AsyncModeProvider has been stabilized. Closing as a result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Data /packages/data [Type] Performance Related to performance efforts
Projects
None yet
Development

No branches or pull requests

2 participants