-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Controlled Inner Blocks Clear On Mount #21804
Comments
I just bisected and found the issue was introduced by #21467. It's a race condition so it was a bit hard to track down. When #21467 introduced a resolver for I see a few solutions:
I need to look deeper into this as I just briefly sleuthed this afternoon. @aduth @youknowriad @Addison-Stavlo What do you think? Does this make sense? Which of the options do you think is best? |
@aduth @youknowriad or maybe we should force the registry to queue listeners in the order that updates happen? We would need to implement store specific subscriptions for that, though. |
Would it actually need to be a measurable duration of time? Or simply be scheduled to occur after the current call stack (i.e. I'd want to take a closer look at the specific implementation, since the flow is hard for me to grasp. I do feel there could be some benefit to making resolvers to be scheduled to start asynchronously, in reducing the chance for these sorts of conflicts, making the behavior more predictable, and reducing reliance on synchronous behaviors. On the other hand, the syncing flow for InnerBlocks has always seemed rather clunky to me. Again, not sure how relevant or possible it would be to refactor, but if it's an option to solve the issue, I think it could be worth exploring. |
The latter is sufficient. I was more worried about the number of tasks scheduled on load when a lot of resolvers fire. I'll put it up on a PR.
Yeah, I imagine there could be other cases where this could be a problem, and intuitively, I expected it to work this way since resolvers are like a forked task.
I agree that could be improved, but perhaps this issue is something we want to fix regardless? |
Awesome! I think the timeout would be a simple and effective solution in that case. Nice job tracking this one down. |
I looked more into this, and the timeout doesn't work because the cause is actually a bit deeper in the code paths. I added comments to the relevant snippet to explain: export function* resetEditorBlocks( blocks, options = {} ) {
const {
__unstableShouldCreateUndoLevel,
selectionStart,
selectionEnd,
} = options;
const edits = { blocks, selectionStart, selectionEnd };
// First Run: `[ core/template-part: [] ], undefined`.
// Second Run: `[ core/template-part: [ ...initialBlocks ] ], false`.
console.log( blocks, __unstableShouldCreateUndoLevel );
if ( __unstableShouldCreateUndoLevel !== false ) {
const { id, type } = yield select( STORE_KEY, 'getCurrentPost' );
const noChange =
( yield select(
'core',
'getEditedEntityRecord',
'postType',
type,
id
) ).blocks === edits.blocks;
// First Run: `false`.
// Second Run: Doesn't reach this code.
// getEditedEntityRecord having a resolver makes
// the execution from the first run reach this point
// after the second run finishes. This overwrites the
// blocks with a stale value. A stale value without
// template part inner blocks which causes the issue
// described here.
console.log( noChange );
if ( noChange ) {
return yield dispatch(
'core',
'__unstableCreateUndoLevel',
'postType',
type,
id
);
}
// We create a new function here on every persistent edit
// to make sure the edit makes the post dirty and creates
// a new undo level.
edits.content = ( { blocks: blocksForSerialization = [] } ) =>
serializeBlocks( blocksForSerialization );
}
yield* editPost( edits );
} A simple solution that I'll implement for now is to make a synchronous But, I think that this problem could surface elsewhere, so the solution should be more general. We should think about canceling stale action execution. That also raises the questions of how to define and continue with any cleanup operations, though. |
Something has regressed with controlled inner blocks where initial values are overwritten with an empty array shortly after mount.
This is noticeable when using the Template Part block placeholder to insert existing template parts. The preview works, but the block is empty after insertion.
The text was updated successfully, but these errors were encountered: