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

"Families" containers #184

Closed
theKashey opened this issue Jan 3, 2023 · 7 comments
Closed

"Families" containers #184

theKashey opened this issue Jan 3, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@theKashey
Copy link

The "global" nature of stores is a feature of sweetstate making it easy to use. It is also some sort of 🔫 footgun creating an opportunity for various issues to appear.

Containers for the rescue, but creating and more importantly using containers for all possible stores is a little complicated, potentially leading to a "wrong coupling" when a parent has to pull containers from different dependencies in order to apply them in a correct place.

I would like to prose an extension to the default Container primitive capable of acting as a boundary for multiple stores, a single Container many:

  • BoundaryContainer: emulates defaultRegistry passing to the one above some stores scoping the rest inside self
    • in a "stop all" mode, it is handy in tests and storybooks to provide more "local" behavior and reduce the necessity to clear defaultRegistry and pollute the global scope.
    • in "stop stores starting with" it is handy around some isolated experience as it limits sub-stores of that experience inside known boundaris

Technical implementation

I can see multiple different ways of achieving desired functionality but would like to propose a family(or a tag) based one.

import {createFamily, BoundaryContainer} from 'react-sweet-state';

const familyA = createFamily('unique-name');

const Store = createStore({
  initialState: {
    count: 0,
  },
  actions: {},
// ⬇️
  family: [familyA], // one can be a part of multiple families, the nearest BoundaryContainer with known family will act as a defaultRegistry
});

// -----

<BoundaryContainer family={familyA} scope="Doe">
  // all stores with corresponding family will "end" here
  <Store />
</FamilyContainer>

Technically speaking, a single FamilyContainer will "unfold" into multiple "normal" ones. And obviously - it can be only a boundary with no life-time hooks present.

Extra requirements

With some stores now being able be "captured" at some point it might be handly to introduce stores which have to be captured. For example, one might demand that a store with configured family should have a dedicated Container to be used as store is "expected" to be isolated.
However, such a feature might be useful for other states as well and ideally should be configurable separately.

@albertogasparin
Copy link
Collaborator

albertogasparin commented Jan 8, 2023

I see 🤔 Interesting idea.
On the naming: what would you think if the feature is called grouping? Like createGroupContainer and createStore({ group: 'bla' }). Or do you think that it might convey a more "deep" integration in the meaning? I find family a bit of an obscure word. Or I quite like tag, assuming we allow multiple to be set.

As for a "stop all" mode, I am not sure I would support it in that same API, as that might open the door to potential mistakes where someone "forgets" to pass a group/tag/family prop and all stores that are supposed to be global suddenly become local.

If we see a particular pain point being the case during testing, exporting a dedicated container (for instance called GlobalContainer) is more explicit and we can easily add it to our own utils.

@albertogasparin albertogasparin added the enhancement New feature or request label Jan 8, 2023
@theKashey
Copy link
Author

Family comes mostly from my background, aka Product Families - something together for any reason, and groups from by point of view demand a better reason causing 🤯
Tags are more explicit here and come without any extra context or meaning - you are right; they seems to be better understood by a wider audience.

The problem here that I am not sure what I really need - I am trying to imagine use case for multiple tags and I can imagine a few theoretical ones (well, multiple expiration tags for cache is a thing, but store is not a resource), but no practical one. A good extension point we might never need.

Speaking of "global" containers - currently, I have the following helper function to be used in tests in order to "check" isolation:

window.getSweetstateGlobalStores = () => {
    return [...defaultRegistry.stores.keys()].filter(name => name.includes('@__global__'));
}

Then I can create snapshot tests to know which stores ended as global, or which states containing a magic keyword are global to make this check less sensitive to changes outside its responsibility.

This is an important moment, as the ability to isolate something without the ability to check and verify is a foot gun 🔫. Probably with a little more love around this aspect we can mitigate all your worries about stop all mode.

@albertogasparin
Copy link
Collaborator

albertogasparin commented Jan 9, 2023

currently, I have the following helper function to be used in tests in order to "check" isolation...

Fair, however that is only due to the lack of this sort of feature. You can still add that function to ensure long term isolation via a specific test, but you would not need to protect production code from it. (or am I missing something else?)

Another thing we could add is an API like

createGlobalContainer({ onInit: (store, otherInitialisedStores) => ... , onUpdate: (store, otherInitialisedStores) => })

where by receiving the store object as argument, you can decide what to do with it. It might be logging, might be dispatching actions, ... Sort of like a middleware, but instead of configuring it globally via the middlewares.add API.

Also thinking more about it, I wonder if the tag/family container API should require the tag on creation and not at runtime. I feel having it as prop might create confusion and need for complex handling (like in case of dynamic tags).

const experienceTag = createTag('my-experience')
const ExperienceContainer = createTagContainer(experienceTag, { onStoreInit, onStoreUpdate });

@theKashey
Copy link
Author

createGlobalContainer is probably how it all should work internally - a "Boundary" which can decide to pass or stop, and then it can be configured in multiple ways to act as one needs.
Technically - the API you've provided can be also used to wire-up connections between different stores without using React components where different stores can currently meet each other.

And speaking about createTagContainer/createTag - the main question is how to retrofit amendment into the current database with less pain. So it should be something not the far from the current concepts.

@albertogasparin
Copy link
Collaborator

albertogasparin commented Jan 11, 2023

a "Boundary" which can decide to pass or stop, and then it can be configured in multiple ways to act as one needs.

Right, so maybe a better API is:

const ExperienceContainer = createGlobalContainer({
  capture: (StoreType) => StoreType.name.includes('experience-'),
  onStoreInit: (storeInstance, otherInitialisedStores) => { ... },
  onStoreUpdate: (storeInstance, otherInitialisedStores) => { ... },
})

Where capture (or filter or ??) is the function that decides which stores should be "registered" by the container. And devs can decide to match stores using name patterns for instance. Just need all experience stores be named experience-bla for instance.

@theKashey
Copy link
Author

Names are handy, but not always the right signal. One need tags to be free to tag some experience- states to be contained, and some other to be not. For example experience-global-cache-lookup can be a global one.

Ideally, it should be more explicitly configurable from the stores itself. Meaning createGlobalContainer + little extension of state info == 💘

@albertogasparin
Copy link
Collaborator

This has been merged as part of the improved Store/Container API #198 .
Basically by using the new containedBy attribute on store creation we can link multiple stores to the same container.

See docs. It will be available on v2.7.0+

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants