Gallery block - Add default value for innerBlockImages #51443
Conversation
geriux
added
[Type] Bug
|
Size Change: +4.59 kB (0%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
|
Hey @glendaviesnz 👋 I added you as a reviewer for this PR but please let me know if you want me to add someone else. Thank you! |
|
For additional context, this appears quite similar to #49557. Can/should we add tests covering this issue? |
Hey @dcalhoun Thanks for sharing that PR. In regards to tests, I haven't found a way to reproduce why the block wouldn't have inner blocks, I guess we could add a test mocking what it is being returned from the Let me know what you think. |
I have been unable to identify the core issue or reproduce the error as well. I agree the errors we are addressing are mere symptoms of a deeper issue. It is a frustratingly elusive issue. A test mocking a return for Additionally, while considering your questions, I further reviewed the errors and the related
WDYT? Footnotes
|
Yeah it is a tricky one 😓
I agree, I'll work on adding this 🙇
The thing is that there are other parts of the code that rely on the value of
I guess this happens when we don't have typed code e.g. Typescript, ideally this hook is expecting its first parameter |
…ctly if there are no inner blocks
|
Flaky tests detected in da45db6. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5289903195
|
Yes, that does make sense. Keeping the current approach sounds good. After considering the error further, I wonder if the issue is not that I do not share that to say we need to postpone merging this fix, but to share the thought that we may be asking the wrong question with "why would a block's |
That's a good question, I do know we have cases where a block could be undefined so it's definitely a larger problem in the project. I'll merge this to see if it solves the current Sentry issues we are getting. |
* Gallery block - Add default value for innerBlockImages * Mobile - Gallery block - Test the block renders the placeholder correctly if there are no inner blocks
What?
This PR adds a default value to
innerBlockImagesin case ofundefinedvalues.Why?
Some parts of the code expect
innerBlockImagesto be defined as well as other values that come from that variable. Some have optional chaining cases but others don't and we are getting some crashes on the mobile editor. Unfourtnaley I couldn't find the exact steps to reproduce the crash but by looking at the code and the stack traces, this code change should address them.How?
By setting a default empty array value to
innerBlockImages.Testing Instructions
Testing Instructions for Keyboard
N/A
Screenshots or screencast
Web
GalleryWeb.mov
Mobile
GalleryMobile.mov