fix: Guard against a block styles crash due to null block values#56903
fix: Guard against a block styles crash due to null block values#56903
Conversation
|
Size Change: 0 B Total Size: 1.71 MB ℹ️ View Unchanged
|
|
Flaky tests detected in d57cf63. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7143982446
|
There was a problem hiding this comment.
Curious - what's the purpose of this idiom in RN over just a [] literal?
There was a problem hiding this comment.
JavaScript does not consider two seemingly equal array literals to be equivalent. I.e., [] === [] returns false.
This becomes important in this particular context as the code in question is a "selector" for querying state in the store. Selectors are a React ecosystem concept that I believe originates from Redux. Selectors provide state to the UI and related logic.
When a selector's return value changes, the UI re-renders for the new value. So, it is a best practice or optimization to avoid returning unnecessary "new" values, which would trigger unnecessary updates to the UI and incur a performance cost.
The Gutenberg project has a development tool that helps avoid introducing unnecessary "new" values from selectors, which helped me ensure I addressed it appropriately. We have a few existing instances to address in the mobile editor.
There was a problem hiding this comment.
Thanks! So the object stays constant, and no change is detected.
In certain scenarios, e.g., when the editor hangs due to poor performance, the `getBlock` value unexpectedly returns `null`. To guard against a crash in this scenario, we now conditionally access attributes on the block. A ideal resolution would be improving editor performance to avoid the scenario where a race condition causes a crash, but the fact remains that `getBlock` _can_ return `null` values and that we should not presume that it will not.
b6da7b2 to
d57cf63
Compare
What?
Guard against
nullvalues returned fromgetBlock.Why?
Fixes wordpress-mobile/gutenberg-mobile#6128. Mitigate
disruptive crashes caused by accessing attributes on a
nullvalue.How?
In certain scenarios, e.g., when the editor hangs due to poor
performance, the
getBlockvalue unexpectedly returnsnull. To guardagainst a crash in this scenario, we now conditionally access attributes
on the block.
A ideal resolution would be improving editor performance to avoid the
scenario where a race condition causes a crash, but the fact remains
that
getBlockcan returnnullvalues and that we should notpresume that it will not.
Additional details are shared in wordpress-mobile/gutenberg-mobile#6128 (comment).
Testing Instructions
The real world scenario where this occurs is difficult to replicate
consistently, as it appears to depend upon a race condition prevalent during
hangs caused by poor app performance. Therefore, a contrived testing approach
is applying the following diff with and without the proposed changes to ensure that
the component correctly handles
nullblock values.Test Diff
Testing Instructions for Keyboard
n/a
Screenshots or screencast
It is worth noting that guarding against the crash results in an unexpected block controls state where the block styles are missing. Hypothetically, the block styles would become visible once the block value is no longer
null. This is unavoidable until the editor performance is addressed and the race condition is avoided.