refactor: Remove non-serializable values in navigation state #53447
Conversation
This file contains hidden or 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
dcalhoun
added
[Type] Code Quality
|
Size Change: 0 B Total Size: 1.75 MB ℹ️ View Unchanged
|
dcalhoun
force-pushed
the
refactor/remove-non-serializable-navigation-state
branch
2 times, most recently
from
a6ab8b3 to
f1c91e7
Compare
|
Flaky tests detected in f1c91e7. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5820681444
|
dcalhoun
force-pushed
the
refactor/remove-non-serializable-navigation-state
branch
from
b1c49fa to
e02adaa
Compare
React Navigation recommends against placing non-serializable values in navigation state as it can break other functionality. https://reactnavigation.org/docs/5.x/troubleshooting#i-get-the-warning-non-serializable-values-were-found-in-the-navigation-state
React Navigation recommends against placing non-serializable values in navigation state as it can break other functionality. https://reactnavigation.org/docs/5.x/troubleshooting#i-get-the-warning-non-serializable-values-were-found-in-the-navigation-state
The filename pattern differed from other screen files by using a period rather than a dash for the "screen" suffix. This difference resulted in unexpected triggers of the CHANGELOG CI task due to this native file not matching the expected native file name pattern matcher: https://github.com/WordPress/gutenberg/blob/3f3c2cd8f8c0bc40160d295823dbb201a8bdd5f1/.github/workflows/check-components-changelog.yml#L8-L14
dcalhoun
force-pushed
the
refactor/remove-non-serializable-navigation-state
branch
from
e02adaa to
d5d6452
Compare
To better align with existing code, the 'Settings' string literal should be replaced with a constant. However, importing the current constant creates a circular dependency. https://github.com/WordPress/gutenberg/blob/12518a05af35aa504f7c8b086cb6f255cf094f25/packages/block-editor/src/components/block-settings/container.native.js#L23
Importing `block-editor` modules from with `components` creates a circular dependency. In the future, this might be further refactored to better align with the web project's organization of focal point picker modules.
Member
Author
|
Closing for now as I do not intend to move this forward in the foreseeable future. |
dcalhoun
deleted the
refactor/remove-non-serializable-navigation-state
branch
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?
Remove non-serializable values from navigation state.
Why?
Fixes #30839. React Navigation recommends against placing non-serializable
values in navigation state as it can break other functionality.
How?
Replace passing children callbacks as navigation parameters with children
passing the new value as parameters when navigating back to the parent screen.
The parent screen relies upon
useEffectto persist any changes to the targetedroute parameter.
Testing Instructions
TBD
Testing Instructions for Keyboard
TBD
Screenshots or screencast
TBD