Data Module: Support parent/child registries

[youknowriad]

Extracted from #14367

The idea of this PR is the possibility to override existing data registries by changing one or multiple stores in it. For example, this is useful when you want to keep using the global stores for viewport, ... but you want to have a separate store for your own block editor instance.

See #14367 for an exemple of use-case.

[nerrad]

So reviewing this and the linked use-case, can you tell me if I'm interpreting things correctly? The purpose of this pull is so that there is now the ability to register a new data registry with another registry as the "parent". Any subsequent stores registering on the new registry that exist on the "parent" will get overridden.

If correct, then I think it'd be useful to see the following:

• A unit-test that demonstrates only the overridden store is modified (i.e. have another store on the parent that is not overridden in the subRegistry registered stores).

Also, I wonder if we'll hit a potential "gotcha" in the future in the case where overridden actions/selectors/reducers for a given parent do not have the same signatures as the parent. In other words what if the parent has the following selectors:

• getFoo( state, someVariable )
• getBar( state, someVarA, someVarB )
• getFooBar( state )

Then let's say a new registry is created and it overrides the parent store, but it's selectors have the following signature:

• getFoo( state )
• getBar( state, someVarA )

Will it matter that the overridden store has no similarity to the parent?

I'm assuming no because the consuming code should be aware of what registry it's using, but for thoroughness I thought it worth asking.

[youknowriad]

A unit-test that demonstrates only the overridden store is modified (i.e. have another store on the parent that is not overridden in the subRegistry registered stores).

I have a unit test showing that the child action/selector are called but not the parent, which is the same thing for me as stores can have different implementations.

Will it matter that the overridden store has no similarity to the parent?

You're right, It doesn't matter I think, I don't think currently see any potential usage for this pattern but I don't see it as problematic. we replace the entire store with its selectors, actions...

[nerrad]

So looking more closely at the implementation example given, the behaviour of this registration is it only affects things interacting with the registry in the new branch right? For example anything in the reusable block component tree will be using the overridden core/block-editor registry separate from what was globally registered higher in the tree? I'm still trying to grasp the implications of this.

[youknowriad]

@nerrad yes, that's correct.

[nerrad]

Thanks for clarifying things for me. Makes sense and looks useful.

[aduth]

Clarifying for myself: It's a bit like extending a registry, correct? Where a new registry is created, and unless a store is overridden, it falls back to the implementation from its immediate parent? The main advantage over something like a clone function considered long ago in #7453 would be that the parent is still "live" (i.e. can have more stores registered later, or state changes applied) with the proposed implementation here? I can see how that would be desirable.

[aduth]

It's nice how simply this works 👍

It may prove to cause #13177 to be more difficult to implement, if we choose to pursue it again in the future (specifically, the change in signature to subscribe( listener[, reducerKeys ] ).

[aduth]

The line prior: We register core/data by default. The store tracks resolutions, which feels like something we should want to be considered globally, maybe represented by only registering it by default for the root registry. What do you think?

[youknowriad]

yes, core/data is proving to be more and more challenging :). to be honest I feel we have two options:

• Have a single global core/data but the question of how to differentiate between stores with similar keys and different registries will be asked.
• Instead of having core/data being a store, having more like a reducer embedded in all stores. There's some backward compatibility concerns with this approach but what I like about it is that it makes the stores even more independent from the registries (related to this comment Try refactoring the reusable blocks to use a separate block editor #14367 (comment))

[nerrad]

Instead of having core/data being a store, having more like a reducer embedded in all stores. There's some backward compatibility concerns with this approach but what I like about it is that it makes the stores even more independent from the registries (related to this comment #14367 (comment))

This approach sounds interesting to me. How would the actions on core/data be exposed (or would there be a special api for the resolution actions/selectors?)

[youknowriad]

How would the actions on core/data be exposed

We could keep them exposed as is (like a virtual store for backwards compatibilty) or have separate actions/selectors for each store (we could even do this today as a shortcut).

If this is something you're interested in exploring, feel free to do so :)

[nerrad]

If this is something you're interested in exploring, feel free to do so :)

I'd love to. Unfortunately my contributions right now are pretty limited to grunt work/feedback/reviews as my client workload has increased. Still I might manage to carve out some personal time to experiment.

[aduth]

The line prior: We register core/data by default. The store tracks resolutions, which feels like something we should want to be considered globally, maybe represented by only registering it by default for the root registry. What do you think?

Noting that in a variant of #14367 with Reusable Block implemented with an embedded <EditorProvider />, this proved to be quite problematic, where saving would not complete because a resolveSelect would never resolve due to the fact that resolution conditions were considered on its own copy of core/data, despite the fact that resolution had occurred on the inherited core store (from the parent, with its own different core/data).

I guess at the very least, hasFinishedResolution would need to be aware of whether the storeKey it receives is one defined in the current registry, or which would be inherited from its parent.

• Have a single global core/data but the question of how to differentiate between stores with similar keys and different registries will be asked.

Depending on whether it's possible and without needing to impact the external API, I wonder if it's a matter of creating some unique identifier per resolver and tracking that as part of resolution status in the "global" core/data. I guess it'd still be challenging in that if a registry was using an inherited store, considering resolution status would have to check its parent registry instead.

[nerrad]

Just throwing out, I wonder if resolution state should be enhanced on registered store state as opposed to global state? That way its specific to the stores?

[aduth]

Just throwing out, I wonder if resolution state should be enhanced on registered store state as opposed to global state? That way its specific to the stores?

Yeah, this sounds similar to what @youknowriad suggested with the embedded reducer. On the note of backwards-compatibility, I don't think it's an issue if the idea is that we still offered selectors from core/data to check resolved status. The problem there is how we'd reimplement those selectors to check the states of the individual stores.

I could see it being a compatibility issue if we're saying that we'd extend the stores with additional selectors like select( 'core/editor' ).hasFinishedResolution, but it wasn't clear to me that's what was proposed, or what's desired.

[aduth]

The line prior: We register core/data by default. The store tracks resolutions, which feels like something we should want to be considered globally, maybe represented by only registering it by default for the root registry. What do you think?

Noting that in a variant of #14367 with Reusable Block implemented with an embedded <EditorProvider />, this proved to be quite problematic, where saving would not complete because a resolveSelect would never resolve due to the fact that resolution conditions were considered on its own copy of core/data, despite the fact that resolution had occurred on the inherited core store (from the parent, with its own different core/data).

Trying to rephrase this, because in re-reading it, I don't think it's very clear:

In the embedded editor, when a save occurs, the implementation of the action will cause a selector call to select( 'core' ).getPostType. Since the core store is not explicitly registered in the child registry, it will be called against that of its parent. The resolver still takes effect, but it checks resolution status against the parent's core/data store. Since getPostType would have been resolved much earlier in the parent store, the resolver never triggers its fetch. From the perspective of the child store, which has its own instance of core/data, the resolution still has not occurred, so it will forever become stuck.

Locally, I resolved this with the following:

diff --git a/packages/data/src/registry.js b/packages/data/src/registry.js
index 9f8268360..2f4bb41bd 100644
--- a/packages/data/src/registry.js
+++ b/packages/data/src/registry.js
@@ -175,10 +175,16 @@ export function createRegistry( storeConfigs = {}, parent = null ) {
 		return registry;
 	}
 
-	Object.entries( {
-		'core/data': dataStore,
-		...storeConfigs,
-	} ).map( ( [ name, config ] ) => registry.registerStore( name, config ) );
+	if ( parent === null ) {
+		storeConfigs = {
+			'core/data': dataStore,
+			...storeConfigs,
+		};
+	}
+
+	Object.entries( storeConfigs ).forEach( ( [ name, config ] ) => {
+		registry.registerStore( name, config );
+	} );
 
 	if ( parent ) {
 		parent.subscribe( globalListener );

But to @youknowriad's point above, the problem becomes that, from core/data, we distinguish on store name and selector, selector args, but notably not registry, so there's a high potential for conflict if two stores in separate registries each have their own need to resolve.

[youknowriad]

Now that #14634 is merged, it seems that we can move forward here