-
Notifications
You must be signed in to change notification settings - Fork 4.6k
iAPI: Return a deep-clone object from getServerState and getServerContext functions
#73437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
iAPI: Return a deep-clone object from getServerState and getServerContext functions
#73437
Conversation
|
Size Change: +10 B (0%) Total Size: 2.54 MB
ℹ️ View Unchanged
|
I would like to confirm if this PR is required for 6.9 RC3, which will be released tomorrow. How urgent is this bug? cc @luisherranz |
|
It should be included. I'm about to start reviewing and merge it if everything is fine. |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
luisherranz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thanks for the fix, David.
I've also added a couple of tests to cover the specific use case of when an object is copied from the server and then attempted to be modified.
|
Flaky tests detected in 116c0ba. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19630995671
|
|
For some reason, the automatic cherry-pick to the 6.9 branch has been skipped. I'll try re-adding the label. |
|
There was a conflict while trying to cherry-pick the commit to the wp/6.9 branch. Please resolve the conflict manually and create a PR to the wp/6.9 branch. PRs to wp/6.9 are similar to PRs to trunk, but you should base your PR on the wp/6.9 branch instead of trunk. |
@DAreRodz @luisherranz, could you manually submit a backport PR against the |
|
Sure. |
…Context` functions (#73437) * Move `deepClone` to utils * Stop using `deepReadOnly` in getServer functions * Update `getServerState` and `getServerContext` e2e tests * Update `getServerState` and `getServerContext` TSDocs * Add changelog update * Remove "read-only" from type tests * Add e2e test for copying obj from server state * Add e2e test for copying obj from server context --------- Co-authored-by: DAreRodz <darerodz@git.wordpress.org> Co-authored-by: luisherranz <luisherranz@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org>
…nd `getServerContext` functions" (#73514) * iAPI: Return a deep-clone object from `getServerState` and `getServerContext` functions (#73437) * Move `deepClone` to utils * Stop using `deepReadOnly` in getServer functions * Update `getServerState` and `getServerContext` e2e tests * Update `getServerState` and `getServerContext` TSDocs * Add changelog update * Remove "read-only" from type tests * Add e2e test for copying obj from server state * Add e2e test for copying obj from server context --------- Co-authored-by: DAreRodz <darerodz@git.wordpress.org> Co-authored-by: luisherranz <luisherranz@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> * Update packages/interactivity/CHANGELOG.md Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com> --------- Co-authored-by: DAreRodz <darerodz@git.wordpress.org> Co-authored-by: luisherranz <luisherranz@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org>
|
This PR was manually backported into the wp/6.9 branch by #73514. |
What?
Fixes a regression with the
getServerStateandgetServerContextfunctions introduced in #72381.The bug makes navigation fail when an object obtained from the returned server state/context is also referenced inside the regular state/context. This worked before without issues.
In particular, the problem happens when the regular state/context is updated during navigation. The algorithm uses an internal function called
deepMerge, which attempts to add new values if they exist. The objects returned bygetServerStateandgetServerContextaredeepReadOnlyobjects, which throw when anything attempts to modify them. If the algorithm tries to add new values to these objects, navigation fails.Why?
This bug is critical. When this occurs, the Interactivity API completely breaks and stops working.
How?
The
getServerStateandgetServerContextfunctions returneddeepReadOnlyas a measure to prevent any modification of these internal objects. The bug fix changes that, making these functions return regular objects without any modification-prevention logic, but cloning the original ones, so the returned values are always the same.Testing Instructions
TBD