Decouple store definition from the registry object #26996
Conversation
youknowriad
requested review from
adamziel,
ajitbohra,
atimmer,
cameronvoell,
draganescu,
ellatrix,
etoledom,
guarani,
kevin940726,
noisysocks,
talldan and
tellthemachines
as code owners
youknowriad
force-pushed
the
update/decouple-store-registry
branch
from
bf89785
to
7221319
Compare
|
Size Change: -155 B (0%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
youknowriad
force-pushed
the
update/decouple-store-registry
branch
from
7221319
to
a068fdc
Compare
|
Should I merge #26655 or do you prefer to merge both PRs at the same time? |
|
@gziolo whatever works for you. If your PR is ready, go for it. |
That's because A related question: once there is only #26866 adds its own |
yes, and we don't want to expose the redux stores in the public APIs.
yes, this is already supported with registerGenericStore (at least it's the intent of that API already)
yes. |
I should precise: the contract with the registry is that it provides the following interface: |
|
About naming: What if "store definition" was called just "store"? Just like we don't have "atom definitions", but only "atoms" and "atom states/values". Then there is a nice symmetry between registries, stores, and atoms: const dataStore = createStore( storeDefinition );
dataRegistry.register( dataStore );
dataRegistry.select( dataStore ).mySelector();
dataRegistry.dispatch( dataStore ).myAction();vs const atom = createAtom( atomDefinition );
atomStore.get( atom );
atomStore.set( atom, value );The Auto-registering stores: In the example above, I have to register a data store manually, but an atom is registered automatically, on first use. Similarly, we could completely remove Or is this forever blocked by the backward compatible API that still supports |
That makes |
|
The renaming makes sense to me.
Good find :) the parallel is very interesting. I believe there's a lot of history here, so we should move with a lot of care but auto-registering stores sound very interesting to me. I'm not sure yet whether we can do it without breaking changes yet. Let's keep this PR at the current stage though of abstraction and consider separately. |
youknowriad
force-pushed
the
try/data-store-as-param
branch
from
ede9269
to
fcd8c78
Compare
youknowriad
force-pushed
the
update/decouple-store-registry
branch
from
5010f80
to
ff4f4f4
Compare
There was a problem hiding this comment.
I've left a few notes focused only on the types. I'll also cite the @dsifford's work that might serve as inspiration: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/wordpress__data/index.d.ts
|
I'm merging this into its parent PR and we'll see there whether we're ready to ship. |
Based on #26655
This takes the "store definition" further by decoupling the store creation from the registration.
Ultimately, this PR means we should deprecate both
registerGenericStoreandregisterStoreAPIs in favor of a uniqueregisterAPI. We can't do that yet because of the two stores we have that rely on "effects middleware" (we need to move these to generators for async behavior to remove that middleware).