Update implementation with feedback

Author: albertogasparin

examples/advanced-shared/index.tsx

@@ -42,9 +42,9 @@ const App = () => (
       <ThemingContainer scope="t1">
         <ThemeHook title="scope" />
       </ThemingContainer>
-      {/* <ThemingContainer initialData={initialData}>
+      <ThemingContainer initialData={initialData}>
         <ThemeHook title="local" />
-      </ThemingContainer> */}
+      </ThemingContainer>
       <ThemingContainer scope="t1">
         <ThemeHook title="scope sync" />
       </ThemingContainer>

src/__tests__/integration.test.js

@@ -469,10 +469,8 @@ describe('Integration', () => {
     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(() => {});
+  it('should throw an error if contained store is used without container', () => {
+    const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
     const Store1 = createStore({
       name: 'one',
       initialState: { todos: [], loading: false },
@@ -481,13 +479,11 @@ describe('Integration', () => {
     });
 
     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');
+    expect(() => render(<Subscriber>{() => null}</Subscriber>)).toThrow(
+      /should be contained/
+    );
+    errorSpy.mockRestore();
   });
 
   describe('dispatchTo', () => {
@@ -514,107 +510,131 @@ describe('Integration', () => {
         },
       });
 
-      const MainContainer = mainContainer ?? (({ children }) => children);
-      const OtherContainer = otherContainer ?? (({ children }) => children);
       const MainSubscriber = createSubscriber(StoreMain);
       const OtherSubscriber = createSubscriber(StoreOther);
       const mainSpy = jest.fn().mockReturnValue(null);
       const otherSpy = jest.fn().mockReturnValue(null);
 
-      const App = ({ plus }) => (
-        <MainContainer>
-          <OtherContainer plus={plus}>
-            <MainSubscriber>{mainSpy}</MainSubscriber>
-            <OtherSubscriber>{otherSpy}</OtherSubscriber>
-          </OtherContainer>
-        </MainContainer>
+      const Content = () => (
+        <>
+          <MainSubscriber>{mainSpy}</MainSubscriber>
+          <OtherSubscriber>{otherSpy}</OtherSubscriber>
+        </>
       );
       return {
-        App,
+        Content,
         StoreMain,
         mainReturn: (n = 0) => mainSpy.mock.calls[n],
         otherReturn: (n = 0) => otherSpy.mock.calls[n],
       };
     };
 
-    it('should allow dispatchTo global -> global', async () => {
-      const { App, mainReturn, otherReturn } = createTestElements({
+    it('should allow dispatchTo global -> global', () => {
+      const { Content, mainReturn, otherReturn } = createTestElements({
         mainContainer: null,
         otherContainer: null,
       });
 
-      render(<App />);
+      render(<Content />);
       const [, mainActions] = mainReturn(0);
       act(() => mainActions.setOther(1));
 
       expect(otherReturn(1)).toEqual([{ count: 1 }, expect.any(Object)]);
     });
 
-    it('should allow dispatchTo contained -> contained', async () => {
+    it('should allow dispatchTo contained -> contained', () => {
       const SharedContainer = createContainer();
-      const { App, mainReturn, otherReturn } = createTestElements({
+      const { Content, mainReturn, otherReturn } = createTestElements({
         mainContainer: SharedContainer,
         otherContainer: SharedContainer,
       });
 
-      render(<App />);
+      render(
+        <SharedContainer>
+          <Content />
+        </SharedContainer>
+      );
       const [, mainActions] = mainReturn(0);
       act(() => mainActions.setOther(1));
 
       expect(otherReturn(1)).toEqual([{ count: 1 }, expect.any(Object)]);
     });
 
-    it('should error when dispatchTo contained -> global', () => {
-      const { App, mainReturn } = createTestElements({
-        mainContainer: createContainer(),
+    it('should allow dispatchTo contained -> global', () => {
+      const MainContainer = createContainer();
+      const { Content, mainReturn, otherReturn } = createTestElements({
+        mainContainer: MainContainer,
         otherContainer: null,
       });
 
-      render(<App />);
+      render(
+        <MainContainer>
+          <Content />
+        </MainContainer>
+      );
       const [, mainActions] = mainReturn(0);
+      act(() => mainActions.setOther(1));
 
-      expect(() => mainActions.setOther(1)).toThrow(/Illegal/);
+      expect(otherReturn(1)).toEqual([{ count: 1 }, expect.any(Object)]);
     });
 
-    it('should error when dispatchTo global -> contained', async () => {
-      const { App, mainReturn } = createTestElements({
-        mainContainer: createContainer(),
-        otherContainer: null,
+    it('should allow dispatchTo global -> contained if properly contained', () => {
+      const OtherContainer = createContainer({ displayName: 'OtherContainer' });
+      const { Content, mainReturn, otherReturn } = createTestElements({
+        mainContainer: null,
+        otherContainer: OtherContainer,
       });
 
-      render(<App />);
+      render(
+        <OtherContainer>
+          <Content />
+        </OtherContainer>
+      );
       const [, mainActions] = mainReturn(0);
+      act(() => mainActions.setOther(1));
 
-      expect(() => mainActions.setOther(1)).toThrow(/Illegal/);
+      expect(otherReturn(1)).toEqual([{ count: 1 }, expect.any(Object)]);
     });
 
-    it('should error when dispatchTo contained -> other contained', async () => {
-      const { App, mainReturn } = createTestElements({
-        mainContainer: createContainer(),
-        otherContainer: createContainer(),
+    it('should allow dispatchTo contained -> other contained', async () => {
+      const MainContainer = createContainer();
+      const OtherContainer = createContainer();
+
+      const { Content, mainReturn, otherReturn } = createTestElements({
+        mainContainer: MainContainer,
+        otherContainer: OtherContainer,
       });
 
-      render(<App />);
+      render(
+        <OtherContainer>
+          <MainContainer>
+            <Content />
+          </MainContainer>
+        </OtherContainer>
+      );
       const [, mainActions] = mainReturn(0);
+      act(() => mainActions.setOther(1));
 
-      expect(() => mainActions.setOther(1)).toThrow(/Illegal/);
+      expect(otherReturn(1)).toEqual([{ count: 1 }, expect.any(Object)]);
     });
 
-    it('should error when dispatchTo override -> contained', async () => {
-      const { App, StoreMain, mainReturn } = createTestElements({
-        mainContainer: null,
-        otherContainer: null,
-      });
+    it('should allow dispatchTo override -> contained', async () => {
+      const { Content, StoreMain, mainReturn, otherReturn } =
+        createTestElements({
+          mainContainer: null,
+          otherContainer: null,
+        });
       const OverrideContainer = createContainer(StoreMain);
 
       render(
         <OverrideContainer>
-          <App />
+          <Content />
         </OverrideContainer>
       );
       const [, mainActions] = mainReturn(0);
+      act(() => mainActions.setOther(1));
 
-      expect(() => mainActions.setOther(1)).toThrow(/Illegal/);
+      expect(otherReturn(1)).toEqual([{ count: 1 }, expect.any(Object)]);
     });
   });
 });

src/components/__tests__/container.test.js

@@ -1,7 +1,7 @@
 /* eslint-env jest */
 
 import React from 'react';
-import { render } from '@testing-library/react';
+import { render, act } from '@testing-library/react';
 
 import { StoreMock, storeStateMock } from '../../__tests__/mocks';
 import { defaultRegistry, StoreRegistry } from '../../store/registry';
@@ -82,7 +82,11 @@ describe('Container', () => {
       const children = <Subscriber>{() => null}</Subscriber>;
       render(<Container scope="s1">{children}</Container>);
 
-      expect(defaultRegistry.getStore).toHaveBeenCalledWith(Store, 's1', true);
+      expect(defaultRegistry.getStore).toHaveBeenCalledWith(
+        Store,
+        's1',
+        expect.any(Function)
+      );
       expect(mockLocalRegistry.getStore).not.toHaveBeenCalled();
     });
 
@@ -104,7 +108,11 @@ describe('Container', () => {
         </Container>
       );
 
-      expect(defaultRegistry.getStore).toHaveBeenCalledWith(Store, 's2', true);
+      expect(defaultRegistry.getStore).toHaveBeenCalledWith(
+        Store,
+        's2',
+        expect.any(Function)
+      );
     });
 
     it('should get local storeState if local matching', () => {
@@ -115,7 +123,7 @@ describe('Container', () => {
       expect(mockLocalRegistry.getStore).toHaveBeenCalledWith(
         Store,
         undefined,
-        true
+        expect.any(Function)
       );
       expect(defaultRegistry.getStore).not.toHaveBeenCalled();
     });
@@ -128,7 +136,7 @@ describe('Container', () => {
       expect(defaultRegistry.getStore).toHaveBeenCalledWith(
         Store,
         undefined,
-        true
+        expect.any(Function)
       );
       expect(mockLocalRegistry.getStore).not.toHaveBeenCalled();
     });
@@ -271,7 +279,7 @@ describe('Container', () => {
       const children = <Subscriber>{renderPropChildren}</Subscriber>;
       render(<Container defaultCount={5}>{children}</Container>);
       const [, { increase }] = renderPropChildren.mock.calls[0];
-      increase();
+      act(() => increase());
 
       expect(actionInner).toHaveBeenCalledWith(expect.any(Object), {
         defaultCount: 5,
@@ -289,7 +297,7 @@ describe('Container', () => {
       );
       const [, { increase }] = renderPropChildren.mock.calls[0];
       rerender(<Container defaultCount={6}>{children}</Container>);
-      increase();
+      act(() => increase());
 
       expect(actionInner).toHaveBeenCalledWith(expect.any(Object), {
         defaultCount: 6,

src/components/__tests__/hook.test.js

@@ -61,7 +61,11 @@ describe('Hook', () => {
     it('should get the storeState from registry', () => {
       const { getRender } = setup();
       getRender();
-      expect(defaultRegistry.getStore).toHaveBeenCalledWith(StoreMock);
+      expect(defaultRegistry.getStore).toHaveBeenCalledWith(
+        StoreMock,
+        undefined,
+        expect.any(Function)
+      );
     });
 
     it('should render children with store data and actions', () => {