-
Notifications
You must be signed in to change notification settings - Fork 4.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
[RNMobile] Flatten inner blocks to fix call stack size exceeded crash #54380
Draft
fluiddot
wants to merge
2
commits into
trunk
Choose a base branch
from
rnmobile/fix/call-stack-size-exceeded-flatten-solution
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
[RNMobile] Flatten inner blocks to fix call stack size exceeded crash #54380
fluiddot
wants to merge
2
commits into
trunk
from
rnmobile/fix/call-stack-size-exceeded-flatten-solution
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fluiddot
added
[Type] Bug
An existing feature does not function as intended
Mobile App - i.e. Android or iOS
Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Mobile App - Automation
Label used to initiate Mobile App PR Automation
labels
Sep 12, 2023
Size Change: 0 B Total Size: 1.52 MB ℹ️ View Unchanged
|
fluiddot
force-pushed
the
rnmobile/fix/call-stack-size-exceeded-flatten-solution
branch
from
September 12, 2023 12:38
9f10634
to
ce38d4d
Compare
Flaky tests detected in ce38d4d. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6159539327
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Mobile App - Automation
Label used to initiate Mobile App PR Automation
Mobile App - i.e. Android or iOS
Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
[Type] Bug
An existing feature does not function as intended
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR expands the block list logic to avoid having deeply nested inner blocks, which leads to crashing the editor. The idea is to identify the group blocks that don’t include any visuals when rendering the inner blocks, and dynamically remove them in the spirit of flattening the content structure.
Why?
Fixes wordpress-mobile/WordPress-Android#18750.
Fixes wordpress-mobile/gutenberg-mobile#6123.
How?
Extract the logic that calculates the block IDs to render in the block list component
gutenberg/packages/block-editor/src/components/block-list/index.native.js
Lines 132 to 137 in ce38d4d
gutenberg/packages/block-editor/src/components/block-list/index.native.js
Line 111 in ce38d4d
This logic hasn't been modified, as it remains as it was before the PR:
gutenberg/packages/block-editor/src/components/block-list/index.native.js
Lines 97 to 101 in 7d054a2
Detect when inner blocks can be flattened
gutenberg/packages/block-editor/src/components/block-list/index.native.js
Lines 123 to 125 in ce38d4d
gutenberg/packages/block-editor/src/components/block-list/index.native.js
Lines 157 to 159 in ce38d4d
gutenberg/packages/block-editor/src/components/block-list/index.native.js
Lines 144 to 155 in ce38d4d
The inner blocks of a block can be flattened if the following conditions are met:
canFlattenInnerBlocks
setting. This is a per-block setting, as the block is the one to determine if its inner blocks can be flattened.gutenberg/packages/block-library/src/group/edit.native.js
Line 143 in ce38d4d
Flatten inner blocks
gutenberg/packages/block-editor/src/components/block-list/index.native.js
Lines 161 to 167 in ce38d4d
This functionality basically iterates through all inner blocks, including the nested ones. If it determines that a block opts to flatten its inner blocks, they will be rendered by the parent.
Testing Instructions
[TBD]
Testing Instructions for Keyboard
N/A
Screenshots or screencast
N/A