atlassian / react-sweet-state

Shared state management solution for React
https://atlassian.github.io/react-sweet-state/
MIT License
871 stars 55 forks source link

Non global states #190

Closed theKashey closed 1 year ago

theKashey commented 1 year ago

continuation of https://github.com/atlassian/react-sweet-state/issues/184

The main difference between SweetState and Context Provider is that the Context requires Provider to exists before Consumer. There is a case without Provider, in such case consumer will get the "default state" of context.

const context = createContext('default');

<Consumer> // 'default'
 <Provider value='overriden'>
  <Consumer> // 'overriden

This creates a lot of friction but also resolves many problems by making information flow unidirectional.

Resolving similar problems in a similar way is something I hope #184 can give us. One day. But I would like to have some support right now.


I would like to introduce temporal mitigation of a problem global states might cause by introducing an extra flag to the createStore making it strict non-global in the dev environment and throw if a boundary container is missing.

const store = createStore<Store, Actions>({
    initialState: {},
    actions: {        
    },
    name: 'my-store',
    isolated: true, // ⬅️ will throw on access to global store
});

There is no other intention for this feature except support refactoring and create "screaming tests" when something which might blow up will blow up. There is no goal to break anything in production, as there could be still use cases missed by tests.

anacierdem commented 1 year ago

There is also another tangent to this: when there is an isGlobal container, RSS will execute the relevant actions (onInit, onUpdate etc.) but if you forget the container (or misplace the hook), it will silently continue using the non-initialized global instance. I think this will also detect container-less usages as once you set isolated to true, you will have to use a container as well. OTOH will only allow a global container, not a scoped one. So maybe we also need a contained: true which will throw on container-less store accesses.

From another viewpoint, one can argue that detecting the existence of a container is possible by additional logic at onInit. Is there a way to detect the same condition (if the instance is global) from the userland code?

theKashey commented 1 year ago

That sounds interesting - distinguish global containers, local containers and container-less use cases.

contained: undefined, // or 'global'
contained: 'local', // local scope-less container
contained: 'scope', // local any scoped container
contained: true // any container, including global

Might be too much for one field, but this looks like a single property. Let's do the opposite and create 3 different options:

// requires container with a scope
scoped: true, // false
// disallow global usage
global: false
// requires any container
contained: true

All 3 makes a sense, but not simultaneously.

albertogasparin commented 1 year ago

Maybe something named expectContainer ? Because I feel like we want to catch problems when some hooks are used outside container but not sure what use case there is for "limiting" a store to be used only on scoped/local 🤔

However, I'm not a huge fun of blowing things up. If we proceed with #184 then it is quite simple to "log" things that end up being global but they should not.

As said in the opening message, this behaviour can be implemented with tag + global container.

const AppContainer = createGlobalContainer({
  capture: (StoreType) => !StoreType.tag.startsWith('global'),
  onStoreInit: (storeInstance, otherInitialisedStores) => { 
    // log exception and report store
  },
})

But I understand this will be less "harsh" than the store self check as it will not catch places where AppContainer is missing. Unless we add a global api to the Registry class:

defaultRegistry.onStoreInit((storeInstance, otherInitialisedStores) => { 
  // log exception and report store
})

That way we can directly look into the stores initialised in the global scope

theKashey commented 1 year ago

Proposed solution:

// alternatively utilize single boolean createStore(
initialState: {}, actions: { }, name: 'my-store', isolated: true, // isolated isolated: false, // global isolated: undefined, // uncontrolled });

- in order not to worry much about the API above - expose constructs with better discoverability. Aka `named factories`:
```tsx
- createLocalStore
- createGlobalStore