From a6d0cbf5f5b5398d0f09b4dfd708f2534e9eef25 Mon Sep 17 00:00:00 2001
From: Alberto Gasparin <agasparin@atlassian.com>
Date: Wed, 17 May 2023 13:46:57 +1000
Subject: [PATCH] Throw error when not contained

---
 docs/api/store.md                            |  3 ++-
 src/components/__tests__/integration.test.js | 21 ++++++++++++++++++++
 src/components/container.js                  |  5 +++--
 src/store/registry.js                        | 16 ++++++++++++---
 4 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/docs/api/store.md b/docs/api/store.md
index a2f49a4..19ee6a8 100644
--- a/docs/api/store.md
+++ b/docs/api/store.md
@@ -16,7 +16,8 @@ createStore(config);
 
    - `name` _(string)_: optional, useful for debugging and to generate more meaningful store keys.
 
-   - `containedBy` _(Container)_: optional, specifies the Container component that should handle the store boundary
+   - `containedBy` _(Container)_: optional, specifies the Container component that should handle the store boundary.  
+     If set, RSS will throw an async uncaught error whenever the store is used without a container, given it might still work but it will likely cause unexpected behaviours.
 
    - `handlers` _(object)_: optional, defines callbacks on specific events
 
diff --git a/src/components/__tests__/integration.test.js b/src/components/__tests__/integration.test.js
index a9eeef3..b35b345 100644
--- a/src/components/__tests__/integration.test.js
+++ b/src/components/__tests__/integration.test.js
@@ -470,4 +470,25 @@ describe('Integration', () => {
     expect(handlers1.onDestroy).toHaveBeenCalledTimes(1);
     expect(handlers2.onDestroy).toHaveBeenCalledTimes(1);
   });
+
+  it('should throw an error if contained store is used without container', async () => {
+    const rejectSpy = jest
+      .spyOn(Promise, 'reject')
+      .mockImplementation(() => {});
+    const Store1 = createStore({
+      name: 'one',
+      initialState: { todos: [], loading: false },
+      actions,
+      containedBy: createContainer(),
+    });
+
+    const Subscriber = createSubscriber(Store1);
+    render(<Subscriber>{() => null}</Subscriber>);
+    await actTick();
+
+    expect(rejectSpy).toHaveBeenCalled();
+    const [error] = rejectSpy.mock.calls[0];
+    expect(error).toEqual(expect.any(Error));
+    expect(error.message).toContain('should be contained');
+  });
 });
diff --git a/src/components/container.js b/src/components/container.js
index 941907e..a6394e5 100644
--- a/src/components/container.js
+++ b/src/components/container.js
@@ -241,7 +241,7 @@ function useContainedStore(scope, registry, props, override) {
       // so we can provide props to actions (only triggered by children)
       if (!containedStore) {
         const isExisting = registry.hasStore(Store, scope);
-        const { storeState } = registry.getStore(Store, scope);
+        const { storeState } = registry.getStore(Store, scope, true);
         const getProps = () => containerProps.current;
         const actions = bindActions(Store.actions, storeState, getProps);
         const handlers = bindActions(
@@ -328,7 +328,8 @@ function createFunctionContainer({ displayName, override } = {}) {
               if (
                 !storeState.listeners().length &&
                 // ensure registry has not already created a new store with same scope
-                storeState === registry.getStore(Store, cachedScope).storeState
+                storeState ===
+                  registry.getStore(Store, cachedScope, true).storeState
               ) {
                 handlers.onDestroy?.();
                 registry.deleteStore(Store, cachedScope);
diff --git a/src/store/registry.js b/src/store/registry.js
index ba1c7c3..ac43e8f 100644
--- a/src/store/registry.js
+++ b/src/store/registry.js
@@ -10,8 +10,18 @@ export class StoreRegistry {
     this.defaultScope = defaultScope;
   }
 
-  initStore = (key, Store) => {
+  initStore = (key, Store, fromContainer) => {
     const { initialState, actions } = Store;
+
+    if (Store.containedBy && !fromContainer) {
+      Promise.reject(
+        new Error(
+          `Store ${Store.key} should be contained by a container but it is used globally. ` +
+            `While it might still work, it will likely cause unexpected behaviours.`
+        )
+      );
+    }
+
     const storeState = createStoreState(key, initialState);
     const boundActions = bindActions(actions, storeState);
     const store = { storeState, actions: boundActions };
@@ -25,9 +35,9 @@ export class StoreRegistry {
     return this.stores.has(key);
   };
 
-  getStore = (Store, scopeId = this.defaultScope) => {
+  getStore = (Store, scopeId = this.defaultScope, fromContainer = false) => {
     const key = this.generateKey(Store, scopeId);
-    return this.stores.get(key) || this.initStore(key, Store);
+    return this.stores.get(key) || this.initStore(key, Store, fromContainer);
   };
 
   deleteStore = (Store, scopeId = this.defaultScope) => {