Skip to content
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

Deprecate registry.use() and update internal implementations #11449

Open
coderkevin opened this issue Nov 3, 2018 · 9 comments
Open

Deprecate registry.use() and update internal implementations #11449

coderkevin opened this issue Nov 3, 2018 · 9 comments
Labels
[Package] Data /packages/data [Type] Code Quality Issues or PRs that relate to code quality

Comments

@coderkevin
Copy link
Contributor

As per the discussion in PR #10289, a follow-up PR is needed to deprecate the registry.use() function in favor of using registerGenericStore instead to implement the functionality needed for internal wp.data plugins.

See the wp.data readme for more details on implementation.

@ocean90 ocean90 added the [Package] Data /packages/data label Nov 3, 2018
@gziolo gziolo added the [Type] Code Quality Issues or PRs that relate to code quality label Apr 24, 2019
@gziolo
Copy link
Member

gziolo commented Apr 24, 2019

As discussed with @coderkevin in private chat I closed initial work in #12004. He is not going to work on it until we have a clear path how to proceed further. As soon as there is an agreement on the final shape of this proposal, he is happy to reopen the PR and help to land it.

@gziolo gziolo added the Needs Decision Needs a decision to be actionable or relevant label Apr 24, 2019
@gziolo
Copy link
Member

gziolo commented May 23, 2019

@youknowriad or @aduth - should we close this issue?

@youknowriad
Copy link
Contributor

I still think it should be done :) but we can discuss it.

@aduth
Copy link
Member

aduth commented May 23, 2019

The introduction of "parent" registries (#14369) has an impact here as well, since it made for some challenges with pluggable behavior taking effect across stores (#14634). In the case of persistence, it may arguably be desirable for a behavior to only take effect on the top-level store. It could probably be implemented differently than the use pattern which exists today, though.

I'm coming around to the idea of deprecating use, but I think we still lack some equivalent to Redux's store enhancers, which it in some ways emulates.

@gziolo gziolo removed the Needs Decision Needs a decision to be actionable or relevant label May 23, 2019
@noahtallen
Copy link
Member

noahtallen commented Mar 15, 2023

I ended up in this issue after a long rabbit hole. registerGenericStore has now been deprecated as well. As far as I can tell, there is no alternative to enabling the persistence plugin? It appears to be de-factor deprecated, since use is (kind of) deprecated, but not really documented anywhere at all.

Essentially, the newer registration methods (createReduxStore + register) aren't supported by the persistence plugin, while registerStore now is. (But that's deprecated)

Looping in @talldan, @jsnajdr, and @adamziel

I brought this up a while ago in a separate conversation, so maybe we can continue thinking about it now!

As far as I can tell, the persistence plugin overrides the registerStore function to provide the initial state out of storage and then subscribe to persist it.

registerStore( storeName, options ) {
if ( ! options.persist ) {
return registry.registerStore( storeName, options );
}
// Load from persistence to use as initial state.
const persistedState = persistence.get()[ storeName ];
if ( persistedState !== undefined ) {
let initialState = options.reducer( options.initialState, {
type: '@@WP/PERSISTENCE_RESTORE',
} );
if (
isPlainObject( initialState ) &&
isPlainObject( persistedState )
) {
// If state is an object, ensure that:
// - Other keys are left intact when persisting only a
// subset of keys.
// - New keys in what would otherwise be used as initial
// state are deeply merged as base for persisted value.
initialState = deepmerge( initialState, persistedState, {
isMergeableObject: isPlainObject,
} );
} else {
// If there is a mismatch in object-likeness of default
// initial or persisted state, defer to persisted value.
initialState = persistedState;
}
options = {
...options,
initialState,
};
}
const store = registry.registerStore( storeName, options );
store.subscribe(
createPersistOnChange(
store.getState,
storeName,
options.persist
)
);
return store;
},
};

This essentially means that the persistence plugin only supports the now deprecated registration method.

Since it seems there was a lot of conversations about deprecating use completely, I'm curious what the path forward should be. I think enabling local storage persistence is worthwhile in wp data. Currently, there is not a documented, un-deprecated way to accomplish that. (The documented ways are all deprecated (but the docs haven't been updated), and while a persistence layer concept was introduced, it's only available in the preferences store.)

Curious for thoughts on this :)

@youknowriad
Copy link
Contributor

I think there's a documented undeprecated way of persisting data. It's the preferences package. It might require some refactoring if you're coming from the old persistence plugin (as it's not automatic) but I think it's a better approach overall.

@noahtallen
Copy link
Member

Right, I did see that. I definitely see how it could replace persistence functionality, but it also doesn't seem like a replacement for full persistence of a redux store. But your thought would be to rewrite any data store using the persistence plugin to instead operate via global app preferences?

@youknowriad
Copy link
Contributor

yes, that's my thinking. The redux store has been initially designed to be an internal implementation detail but the "use" API kind of defeated that purpose.

@jsnajdr
Copy link
Member

jsnajdr commented Mar 16, 2023

Another way how to make the persistence plugin compatible with the register and createReduxStore functions, is to convert it to a Redux store enhancer, and allow enhancers (and custom middlewares) to be passed as one of the options for createReduxStore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Data /packages/data [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants