Migrate post editor panels state to preferences store
#39604
Merged
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
talldan
added
[Type] Enhancement
talldan
requested review from
ajitbohra,
juanmaguitar,
mkaz,
nerrad and
ryanwelcher
as code owners
talldan
changed the title
panels state to preferences store
18 tasks
|
Size Change: +182 B (0%) Total Size: 1.21 MB
ℹ️ View Unchanged
|
|
|
||
| // Panels were a preference, but the data structure changed when the state | ||
| // was migrated to the preferences store. They need to be converted from | ||
| // the old to new format. |
Contributor
There was a problem hiding this comment.
This is a little confusing as it says "They need to be converted from the old to new format", but they are passed through a method called ToOld format?
Contributor
There was a problem hiding this comment.
What is the context that causes the need to have them converted back to the old format? I am having trouble trying to work out what combination of editor version versus stored data would need that - apologies if I am overlooking the obvious.
Contributor
Author
There was a problem hiding this comment.
Ah, that looks like a typo, thanks for catching it. I'll fix that 😄
What is the context that causes the need to have them converted back to the old format?
It's purely if a plugin is using the getPreference and/or getPreferences selector for panel data, it would be a breaking change to suddenly provide that data in the new format. So while it's painful, I think the best thing to do is convert to the old format. The good thing is these selectors are now deprecated, so plugins should be moving away from using them.
Contributor
There was a problem hiding this comment.
That makes sense - I completely overlooked plugins when trying to think this through!
Contributor
|
This worked as advertised for me for both migrating old format to the new, and modifying panels in new format. The only thing I noticed, and it probably isn't important, that is a panel can be in both the |
Contributor
Author
|
Thanks for reviewing @glendaviesnz. I've fixed that typo in the comment.
Yeh, it makes sense - it only looks odd if you are inspecting the data object, but one would hope that not too many users do that 😄 |
talldan
force-pushed
the
migrate/post-editor-panels-state-to-preferences-store
branch
from
0e55dba to
d8dac3b
Compare
talldan
deleted the
migrate/post-editor-panels-state-to-preferences-store
branch
talldan
changed the title
panels state to preferences storepanels state to preferences store
talldan
changed the title
panels state to preferences storepanels state to preferences store
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.
Part of #31965
What?
Migrates the post editor
panelsdata to the preferences store.This also deprecates and removes a few things.
The
getPreferenceandgetPreferencesselectors in the post editor are both deprecated, as they no longer need to be used, but as stable APIs backwards compatibility is needed.The entire
preferencesreducer is also no longer needed, so that can be removed and the store switched over to usingregisterinstead ofregisterStorenow that there's no persistence.Why?
As outlined in #31965, the goal is to move persisted data out of our various stores to a centralized preferences store.
When doing this, I decided to change the data structure. The original data structure involved some heavily nested data, it wasn't normalized in the way redux style state often is, and that makes it complicated to work with.
There were some problems as well in that the existing data structure requires serializing and persisting far more data than is necessary. Each panel defaults to
true, and yet the reducer still stores and persistsenabled: true. Each panel is also notopenedby default, and yet it storesopened: false. In both these cases, the absence of a value is perfectly adequate.In the new data structure,
enabledhas been flipped to be stored in aninactivePanelspreference.How?
This follows a familiar process to other migration PRs. The selectors and actions have been rewired to use the preferences store.
The only difference is the conversion of the data into a different structure. This is a bit ugly in the
getPreferenceselector, but that selector is now deprecated and unused, so I don't think it's a big issue.Testing Instructions
trunkand build