[RNMobile] performance improvements #27446
Merged
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
|
Size Change: 0 B Total Size: 1.36 MB ℹ️ View Unchanged
|
gziolo
added
Mobile App - i.e. Android or iOS
|
Hello @dratwas, during testing I've discovered a regression on your branch, please look on it:
|
Thank you @lukewalczak for reporting this. I fixed it, could you please try it with the latest changes? |
lukewalczak
approved these changes
Feb 2, 2021
There was a problem hiding this comment.
Code changes LGTM! Also the previously mentioned issue is fixed. I have tested touched blocks on both platforms. Thanks @dratwas!
2 tasks
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
In this PR I added some performance fixes that I found during my performance audit.
gutenberg-mobile PR wordpress-mobile/gutenberg-mobile#3088
The
rootBlockIdwas declared after theblockCountso we always used undefined as an argument ingetBlockCounthttps://github.com/WordPress/gutenberg/pull/27446/files#diff-753322c95666679508eecb52371e38647190768c3198f16c1dc312a57c5b0e41R346
The block variation picker is rendered each time something changes in the store, thanks to the
useMemowe avoid unnecessary re-rendershttps://github.com/WordPress/gutenberg/pull/27446/files#diff-c5fca3838ebdbf49b8a75d83c2da3706e812c3fe400c36bf0bbfe3f0c6477145R76
The
isAnyBlockSelectedshould be boolean but sometimes it is a block because of this assignmenthttps://github.com/WordPress/gutenberg/pull/27446/files#diff-c3e8e7a2a2d154022e4803ff7007fdedda65e535389a9dbbfa320e037cbfe4dcR293
I just cast it to the boolean and thanks to that we avoid unnecessary renders
https://github.com/WordPress/gutenberg/pull/27446/files#diff-c3e8e7a2a2d154022e4803ff7007fdedda65e535389a9dbbfa320e037cbfe4dcR351
The button block uses a
selectedIdas a prop which forces the render of every button when we change selection.It was not only re-render but also an execution of
onToggleButtonFocuswhich callsetStateinside.https://github.com/WordPress/gutenberg/pull/27446/files#diff-982d4f7ade1829603ee66453ddd8d0aa0a8fa416b82917911d07a7fb924695b6R75
I found that we need only info about that if specific button
isSelectedso i removed theselectedIdand use theisSelectedinsteadhttps://github.com/WordPress/gutenberg/pull/27446/files#diff-982d4f7ade1829603ee66453ddd8d0aa0a8fa416b82917911d07a7fb924695b6R160
I also added a check to prevent of calling
setStatewhen the value is the samehttps://github.com/WordPress/gutenberg/pull/27446/files#diff-982d4f7ade1829603ee66453ddd8d0aa0a8fa416b82917911d07a7fb924695b6R160
The
isEditorSidebarOpenedchanges each time we open/close the block settings bottom sheet. If we use it as a prop then it forces re-render even if we don't need to (the block is not selected). I added a check to use theisEditorSidebarOpenedonly if block is selectedhttps://github.com/WordPress/gutenberg/pull/27446/files#diff-982d4f7ade1829603ee66453ddd8d0aa0a8fa416b82917911d07a7fb924695b6R430
I moved the
layoutPropto const to not generate new object on every renderhttps://github.com/WordPress/gutenberg/pull/27446/files#diff-4347ded500d1f93237e9e254af2e2f5a01896b5d1736c61f965f6d333847f8bdR27
I also used
useCallbackto avoid of passing new function reference to childrenhttps://github.com/WordPress/gutenberg/pull/27446/files#diff-4347ded500d1f93237e9e254af2e2f5a01896b5d1736c61f965f6d333847f8bdR79
https://github.com/WordPress/gutenberg/pull/27446/files#diff-4347ded500d1f93237e9e254af2e2f5a01896b5d1736c61f965f6d333847f8bdR114
https://github.com/WordPress/gutenberg/pull/27446/files#diff-9c40c4e10ac9a0506fba110cc380f3ffa8debc0b2ab65fe6501c9615e0169c3aR453
I did the same with file block as in case of button block. I added check if block is selected to the
isEditorSidebarOpenedhttps://github.com/WordPress/gutenberg/pull/27446/files#diff-6a1044ed83415e71b054aaebbee6a525cd1c95f0f45b8328312378c842e57f30R583
I removed not used props in the media-text block
https://github.com/WordPress/gutenberg/pull/27446/files#diff-61739aa4e859ae0d5534c4b2caf2bce5df830b32585a2d6e6aa54ccca43fa225L395
I added
useCallbackto the paragraph block to avoid of creating new references on every renderhttps://github.com/WordPress/gutenberg/pull/27446/files#diff-61739aa4e859ae0d5534c4b2caf2bce5df830b32585a2d6e6aa54ccca43fa225L395
The link picker called
setAttributeseven if the attributes were the same which forced re-render of all providers etc. I added some check to avoid ithttps://github.com/WordPress/gutenberg/pull/27446/files#diff-1c6b716518b93eef13a216ed21fa8d92122f8941fbac8378f13f73a4ae5314dcR152
I added a check in the
layout.jsto avoid setting state with the same valuehttps://github.com/WordPress/gutenberg/pull/27446/files#diff-f687f5c0996917c65cac644a7e5bd220c61f4d81fb9dc2449fe522cf8bb33c1bR83
I added a babel plugin that will remove all
console[logs/warns]in the production bundleshttps://github.com/WordPress/gutenberg/pull/27446/files#diff-6838f0ad1bf0f202fcb60582f166bd8dc57a53388f9a194b85c5c068b4bd6906R57
I tested it and seems like something has changed because the
onSelectionchangeis called when we select the paragraph and this comment is not valid. The selected paragraph was rencered two times because we called theonSelectionChangewith 0/0 or old values and then with the correct value (from Aztec). I removed this code since i couldn't find a scenario when it would be needed. Thanks to that we do not dispatch 2 actions but only one.https://github.com/WordPress/gutenberg/pull/27446/files#diff-7af18df9c3e07e67518894550a2e1d9d627feb879a2003a2e231ef927d4d06f8L704
I believe that this is the most important change but it is really small. Please note that:
NEVER do something like this in the useSelect
Because of that, every rich-text is rendered on each change in the redux-store. Imagine that we have a post with 100 rich text and all of them are re-rendered if we type something in one of them. And it is just because of
|| {}:)https://github.com/WordPress/gutenberg/pull/27446/files#diff-7af18df9c3e07e67518894550a2e1d9d627feb879a2003a2e231ef927d4d06f8L704
How has this been tested?
The app should run a bit faster - easier to observe with large posts
Types of changes
I added some performance fixes
Checklist: