Blocks: simplify/optimise isUnmodifiedBlock #56919
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: -32 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 4e48a90. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7179834066
|
ellatrix
force-pushed
the
simplify/unmodified-default-block
branch
from
cf61631
to
a1a7015
Compare
|
Initial testing seems good at the Gutenberg previewer. |
ellatrix
commented
Dec 10, 2023
ellatrix
added
[Type] Performance
dmsnell
reviewed
Dec 10, 2023
| function isEqual( a, b ) { | ||
| return ( a?.valueOf() ?? a ) === ( b?.valueOf() ?? b ); | ||
| } | ||
| return Object.entries( getBlockType( block.name )?.attributes ?? {} ).every( |
There was a problem hiding this comment.
we're checking the attributes on the block type here, but would it be important to compare also the attributes on this particular block instance that may not be on the block type?
for instance, if I setAttribute('formerlyNotPresentAttribute', true) then I would imagine I have a modified block, but the block type won't have formerlyNotPresentAttribute so it won't show different here
There was a problem hiding this comment.
We didn't before. And I guess if it's not officially declared, why check?
There was a problem hiding this comment.
I missed that in the before code. makes me wonder anyway if it's been a latent bug. no need to make it part of this change.
ellatrix
force-pushed
the
simplify/unmodified-default-block
branch
from
a1a7015
to
4e48a90
Compare
youknowriad
approved these changes
Dec 13, 2023
|
Yes, the only suspects are block popover and the inserter/appender. But block popover doesn't show when typing, though we might be calling those selectors. We might also be calling appender selectors. Might have a look at some point. |
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.
What?
This is one of the functions I see pop up a lot in the performance graph for the
inputevent, due to the rich text value to store PR (#43204). Because we're now comparing.valueOf(), these attributes are now serialised to check strict equality.This PR changes to function to look at the defaults in the attribute definition. If the attributes don't match the defaults, or are not empty, it means the block was modified.
Why?
This function runs on typing so it's important to change. It's also simpler.
How?
Described above
Testing Instructions
No tests should fail.
Testing Instructions for Keyboard
Screenshots or screencast