Fix hooks with side effects in strict mode; add tests

.eslintrc.js

@@ -35,6 +35,10 @@ module.exports = {
     '@typescript-eslint/no-use-before-define': ['error'],
     'react-hooks/rules-of-hooks': 'error', // Checks rules of Hooks
     'react-hooks/exhaustive-deps': 'warn', // Checks effect dependencies
-    'react/react-in-jsx-scope': 'off', // suppress errors for missing 'import React' in files
+    'no-unused-vars': 'off',
+    '@typescript-eslint/no-unused-vars': [
+      'error',
+      { argsIgnorePattern: '^_', varsIgnorePattern: '^_' },
+    ],
   },
 };

example/src/App.tsx

@@ -1,55 +1,28 @@
-import React, { useEffect } from 'react';
-import {
-  makeReactive,
-  useReactive,
-  useWatch,
-  useWatchEffect,
-} from '@hlysine/reactive';
+import React, { useState } from 'react';
+import { makeReactive, reactive, useWatchEffect } from '@hlysine/reactive';
 
-export default makeReactive(function App() {
-  const obj = useReactive(() => ({
-    nested1: {
-      a: 1,
-    },
-    nested2: {
-      b: 1,
-    },
-    first: true,
-  }));
-
-  useEffect(() => {
-    console.log('App mounted');
-    return () => console.log('App unmounted');
-  }, []);
+const obj = reactive({ a: 1, b: 2 });
 
+const Watcher = makeReactive(function Watcher() {
+  const [, setState] = useState(0);
   useWatchEffect(() => {
-    console.log('watch effect triggered', obj.nested1.a);
-    return () => console.log('watch effect cleanup');
+    console.log('watchEffect', obj.a);
+    setState(obj.a);
+    return () => console.log('cleanup useWatchEffect');
   });
+  console.log('render App');
 
-  useWatch(
-    obj,
-    (newVal, oldVal) => {
-      console.log('watch triggered', oldVal, newVal);
-      return () => console.log('watch cleanup');
-    },
-    { immediate: true }
-  );
+  return <button onClick={() => obj.a++}>Test update inside watcher</button>;
+});
+
+export default makeReactive(function App() {
+  const [show, setShow] = useState(true);
 
   return (
     <>
-      <button type="button" onClick={() => (obj.first = !obj.first)}>
-        {obj.first ? 'first' : 'second'}
-      </button>
-      {obj.first ? (
-        <button type="button" onClick={() => obj.nested1.a++}>
-          {obj.nested1.a}
-        </button>
-      ) : (
-        <button type="button" onClick={() => obj.nested2.b++}>
-          {obj.nested2.b}
-        </button>
-      )}
+      <button onClick={() => setShow((v) => !v)}>show watcher</button>
+      <button onClick={() => obj.a++}>Test update</button>
+      {show && <Watcher />}
     </>
   );
 });

jest.config.js

@@ -1,8 +1,8 @@
 module.exports = {
   preset: 'ts-jest',
   testEnvironment: 'jsdom',
-  setupFiles: ['<rootDir>/setupTests.ts'],
-  testPathIgnorePatterns: ['example', 'node_modules'],
+  setupFiles: ['<rootDir>/src/__tests__/__utils__/setup.ts'],
+  testPathIgnorePatterns: ['example', 'node_modules', '/__utils__/'],
   moduleNameMapper: {
     '\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$':
       '<rootDir>/__mocks__/fileMock.js',

package.json

@@ -28,7 +28,7 @@
     "make:docs": "typedoc",
     "lint": "eslint src",
     "test": "jest --coverage",
-    "test:watch": "jest --coverage --watch",
+    "test:watch": "jest --watch",
     "predeploy": "typedoc",
     "deploy": "gh-pages -d docs",
     "prepare": "husky install"

src/__tests__/__utils__/helper.tsx

@@ -0,0 +1,51 @@
+import React from 'react';
+import {
+  Queries,
+  RenderHookOptions,
+  RenderHookResult,
+  render,
+} from '@testing-library/react';
+import { queries } from '@testing-library/dom';
+
+export function renderReactiveHook<
+  Result,
+  Props,
+  Q extends Queries = typeof queries,
+  Container extends Element | DocumentFragment = HTMLElement,
+  BaseElement extends Element | DocumentFragment = Container
+>(
+  renderCallback: (initialProps: Props) => Result,
+  options: RenderHookOptions<Props, Q, Container, BaseElement> = {}
+): RenderHookResult<Result, Props> {
+  const { initialProps, ...renderOptions } = options;
+  const result = React.createRef<Result>() as {
+    current: Result;
+  };
+
+  function TestComponent({
+    renderCallbackProps,
+  }: {
+    renderCallbackProps: Props;
+  }) {
+    const pendingResult = renderCallback(renderCallbackProps);
+
+    React.useEffect(() => {
+      result.current = pendingResult;
+    });
+
+    return null;
+  }
+
+  const { rerender: baseRerender, unmount } = render(
+    <TestComponent renderCallbackProps={initialProps!} />,
+    renderOptions
+  );
+
+  function rerender(rerenderCallbackProps) {
+    return baseRerender(
+      <TestComponent renderCallbackProps={rerenderCallbackProps} />
+    );
+  }
+
+  return { result, rerender, unmount };
+}

src/__tests__/component.test.tsx

@@ -1,11 +1,11 @@
 import React from 'react';
 import { act, render } from '@testing-library/react';
 import '@testing-library/jest-dom';
-import { makeReactive, ref, useWatchEffect } from '..';
+import { makeReactive, reactive, ref, useComputed, useWatchEffect } from '..';
 import { perf, wait } from 'react-performance-testing';
 import 'jest-performance-testing';
 
-describe('Reactive component', () => {
+describe('makeReactive', () => {
   it('renders without crashing', async () => {
     const Tester = makeReactive(function Tester() {
       return <p>Test component</p>;
@@ -40,6 +40,90 @@ describe('Reactive component', () => {
       expect(content).toBeTruthy();
     });
   });
+  it('batches state updates', async () => {
+    const obj = reactive({ a: 1, b: 2 });
+    const Tester = makeReactive(function Tester() {
+      return (
+        <p data-testid="obj">
+          a: {obj.a}, b: {obj.b}
+        </p>
+      );
+    });
+
+    const { renderCount } = perf(React);
+
+    const { findByTestId } = render(<Tester />);
+
+    const content = await findByTestId('obj');
+    expect(content.innerHTML).toContain('a: 1, b: 2');
+
+    act(() => {
+      obj.a++;
+      obj.b++;
+    });
+
+    await wait(async () => {
+      // should re-render with updated value
+      expect(renderCount.current.Tester).toBeRenderedTimes(2);
+      const content = await findByTestId('obj');
+      expect(content.innerHTML).toContain('a: 2, b: 3');
+    });
+  });
+  it('does not re-render when unrelated state changes', async () => {
+    const obj = reactive({ a: 1, b: 2 });
+    const Tester = makeReactive(function Tester() {
+      return <p>{obj.a}</p>;
+    });
+
+    const { renderCount } = perf(React);
+
+    const { findByText } = render(<Tester />);
+
+    const content = await findByText('1');
+    expect(content).toBeTruthy();
+
+    act(() => {
+      obj.b++;
+    });
+
+    await wait(async () => {
+      // should not re-render
+      expect(renderCount.current.Tester).toBeRenderedTimes(1);
+      const content = await findByText('1');
+      expect(content).toBeTruthy();
+    });
+  });
+  it('does not re-render when state changes in nested watcher', async () => {
+    const obj = reactive({ a: 1, b: 2 });
+    const mockEffect = jest.fn();
+
+    const Tester = makeReactive(function Tester() {
+      useWatchEffect(() => {
+        mockEffect(obj.b);
+      });
+      return <p>{obj.a}</p>;
+    });
+
+    const { renderCount } = perf(React);
+
+    const { findByText } = render(<Tester />);
+
+    expect(mockEffect).toBeCalledTimes(1);
+    const content = await findByText('1');
+    expect(content).toBeTruthy();
+
+    act(() => {
+      obj.b++;
+    });
+
+    await wait(async () => {
+      // should not re-render, but should trigger effect again
+      expect(renderCount.current.Tester).toBeRenderedTimes(1);
+      expect(mockEffect).toBeCalledTimes(2);
+      const content = await findByText('1');
+      expect(content).toBeTruthy();
+    });
+  });
   it('transfers component attributes correctly', () => {
     const propTypes = {};
     const contextTypes = {};
@@ -62,43 +146,50 @@ describe('Reactive component', () => {
     expect(converted.name).toBe(Tester.name);
   });
   it('stops reactive effects on unmount', async () => {
+    const count = ref(0);
+
     const mockEffect = jest.fn();
     const mockCleanup = jest.fn();
-    const count = ref(0);
+    const mockGetter = jest.fn(() => count.value + 1);
     const Tester = makeReactive(function Tester() {
       useWatchEffect(() => {
         mockEffect(count.value);
         return mockCleanup;
       });
-      return <p>{count.value}</p>;
+      const derived = useComputed(mockGetter);
+      return <p>{derived.value}</p>;
     });
 
     const { unmount, findByText } = render(<Tester />);
 
-    expect(mockEffect.mock.calls).toHaveLength(1);
-    expect(mockCleanup.mock.calls).toHaveLength(0);
-    const content1 = await findByText('0');
+    expect(mockEffect).toBeCalledTimes(1);
+    expect(mockCleanup).toBeCalledTimes(0);
+    expect(mockGetter).toBeCalledTimes(1);
+    const content1 = await findByText('1');
     expect(content1).toBeTruthy();
 
     act(() => {
       count.value++;
     });
 
-    expect(mockEffect.mock.calls).toHaveLength(2);
-    expect(mockCleanup.mock.calls).toHaveLength(1);
-    const content2 = await findByText('1');
+    expect(mockEffect).toBeCalledTimes(2);
+    expect(mockCleanup).toBeCalledTimes(1);
+    expect(mockGetter).toBeCalledTimes(2);
+    const content2 = await findByText('2');
     expect(content2).toBeTruthy();
 
     unmount();
 
-    expect(mockEffect.mock.calls).toHaveLength(2);
-    expect(mockCleanup.mock.calls).toHaveLength(2);
+    expect(mockEffect).toBeCalledTimes(2);
+    expect(mockCleanup).toBeCalledTimes(2);
+    expect(mockGetter).toBeCalledTimes(2);
 
     act(() => {
       count.value++;
     });
 
-    expect(mockEffect.mock.calls).toHaveLength(2);
-    expect(mockCleanup.mock.calls).toHaveLength(2);
+    expect(mockEffect).toBeCalledTimes(2);
+    expect(mockCleanup).toBeCalledTimes(2);
+    expect(mockGetter).toBeCalledTimes(2);
   });
 });