The useHook with selector does not trigger rendering properly

[bcytrowski]

When I use the useHook with selector the rendering of component using it is not being triggered. It looks like the state returned from selector is being compared 1 level deep to figure out if it is structurally identical to the previous one. I expect it to trigger rendering when the object returned from selector is referentially different than the previously returned one.

I checked all versions of react-sweet-state (since v1.0.2) and this is an issue for all of them.

This would trigger rendering, which is fine:

let currentState = { x: 10 };
let nextState = { x: 12 };

const selector = state => state.x

This won't :(

let currentState = { x: { name: "John" } };
let nextState = { x: { name: "John" } };

const selector = state => state.x

Fun part - this will trigger rendering too (which is fine):

let currentState = { x: { fakeLevel: { name: "John" } } };
let nextState =  { x: { fakeLevel: { name: "John" } } };

const selector = state => state.x

Below is the extra test case patch you can apply on the codebase to see the difference in number of renders for useHook with and without selector for the state which consists of only one field. To make the test pass change the actions accordingly:

actions: {
+        setLastChange:
+          (fieldId) =>
+          ({ setState }) => {
-            setState({ lastChange: { changedFieldId: fieldId } });
+            setState({ lastChange: { fakeLevel: { changedFieldId: fieldId } } });
+          },
+      },

diff --git a/src/components/__tests__/integration.test.js b/src/components/__tests__/integration.test.js
index 0e3b513..afcf41f 100644
--- a/src/components/__tests__/integration.test.js
+++ b/src/components/__tests__/integration.test.js
@@ -47,6 +47,60 @@ describe('Integration', () => {
     defaultRegistry.stores.clear();
   });

+  fit('should trigger rendering when selector returns referencially different value', async () => {
+    const Store = createStore({
+      initialState: { lastChange: null },
+      actions: {
+        setLastChange:
+          (fieldId) =>
+          ({ setState }) => {
+            setState({ lastChange: { changedFieldId: fieldId } });
+          },
+      },
+    });
+
+    const Container = createContainer(Store);
+    const useHook = createHook(Store);
+    const useLastChange = createHook(Store, {
+      selector: (state) => state.lastChange,
+    });
+
+    const notifyNoSelector = jest.fn();
+    const notify = jest.fn();
+
+    const ChangeNotifier = () => {
+      const lastChange = useLastChange();
+      notify(lastChange);
+      return null;
+    };
+
+    let acts;
+
+    const Hook = () => {
+      const [state, boundActions] = useHook();
+      acts = boundActions;
+      notifyNoSelector(state);
+      return <div>Nothing</div>;
+    };
+
+    render(
+      <Container>
+        <ChangeNotifier />
+        <Hook />
+      </Container>
+    );
+    expect(notifyNoSelector).toHaveBeenCalledTimes(1);
+    expect(notify).toHaveBeenCalledTimes(1);
+
+    act(() => acts.setLastChange('summary'));
+    expect(notifyNoSelector).toHaveBeenCalledTimes(2);
+    expect(notify).toHaveBeenCalledTimes(2);
+
+    act(() => acts.setLastChange('summary'));
+    expect(notifyNoSelector).toHaveBeenCalledTimes(3);
+    expect(notify).toHaveBeenCalledTimes(3);
+  });
+
   it('should get closer storeState with scope id if matching', () => {
     const Container = createContainer(Store);
     const Subscriber = createSubscriber(Store);

[anacierdem]

This is by design. See the selector docs here as well. The output of the selectors undergo a shallow equality check. I also missed this previously when I opened this issue and proposed that maybe exposing the equality function might be useful. On the other hand at this point I'm pretty much convinced that keeping it opinionated is safer for the health of the library.

[albertogasparin]

From a dev expectation pov, 99% of the times selectors returning same data wrapped in a different object instance mean nothing has changed. So a re-render is pointless. I understand that there might be cases where this behaviour is unwanted, but the workaround is fairly simple and cheap (as you described, just add a nested object) that I feel expanding the API has not much value. However if you feel like docs should make a better job highlighting this behaviour, I welcome suggestions 😉

[bcytrowski]

I totally agree with you guys - I just shared my observations of a dev coming from redux/reselect world. I believe aligning the current behavior with react-redux useSelector would be a breaking one. I'm totally fine with the outcome of this thread and the workaround is perfect for me :) Thanks