WIP: Chore/map hooks

[tomwayson]

Using hooks for the map component has proved a bit of a challenge.

My first pass (2b9831d) worked, but I noticed that I was creating a new map each time the items changed, which is not desirable, nor how the original class component behaved.

In 2b9831d I solved that by breaking the code inside useEffect() into 2 different effects, one that only runs on mount/unmount, and the other that runs every time the items prop is updated. That appeared to work well in the app, but then tests were failing and showing these warnings:

    console.error node_modules/react-dom/cjs/react-dom.development.js:506
      Warning: An update to ExtentsMap inside a test was not wrapped in act(...).
      
      When testing, code that causes React state updates should be wrapped into act(...):
      
      act(() => {
        /* fire events that update state */
      });
      /* assert on the output */
      
      This ensures that you're testing the behavior the user would see in the browser. Learn more at https://fb.me/react-wrap-tests-with-act
    console.error node_modules/react-dom/cjs/react-dom.development.js:506
      Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.
          in ExtentsMap (at ExtentsMap.test.js:15)
    console.error node_modules/react-dom/cjs/react-dom.development.js:506
      Warning: An update to ExtentsMap inside a test was not wrapped in act(...).
      
      When testing, code that causes React state updates should be wrapped into act(...):
      
      act(() => {
        /* fire events that update state */
      });
      /* assert on the output */
      
      This ensures that you're testing the behavior the user would see in the browser. Learn more at https://fb.me/react-wrap-tests-with-act

I would guess that react-testing-library was already doing that for me, but maybe not so in 2f12db6 I explicitly tried using act(). I commented out the second test, and got the first test to pass, but I still see the Warning: An update to ExtentsMap inside a test was not wrapped in act(...). warning.

[tomwayson]

FYI - I didn't even run tests on the first commit until now, and I see that the assertion that the newMap() mock was called failed, though newMap() was clearly called:

    console.log src/components/ExtentsMap.js:13
      newMap
    console.log src/components/ExtentsMap.js:26
      unmounting
    console.log src/components/ExtentsMap.js:16
      refreshGraphics function
    console.log src/components/ExtentsMap.js:13
      newMap
    console.log src/components/ExtentsMap.js:16
      refreshGraphics function
    console.log src/components/ExtentsMap.js:26
      unmounting

  ● components › ExtentsMap › should render with no items

    expect(received).toBe(expected) // Object.is equality

    Expected: 1
    Received: 0

      17 |       expect(getByTestId('map')).toBeInTheDocument();
      18 |       // validate that newMap() was called w/ a DOM node and map options
    > 19 |       expect(newMap.mock.calls.length).toBe(1);
         |                                        ^
      20 |       const newMapArgs = newMap.mock.calls[0];
      21 |       expect(newMapArgs[0]).toBeInstanceOf(HTMLDivElement);
      22 |       expect(newMapArgs[1]).toEqual({

      at Object.toBe (src/components/ExtentsMap.test.js:19:40)

[tomwayson]

Maybe try either:

• using useLayoutEffect() or
• using react-testing-library's flushEffects() (perhaps instead of wait?)

See: https://blog.kentcdodds.com/react-hooks-whats-going-to-happen-to-my-tests-df4c2b4d67b7

[tomwayson]

After upgrading react-testing-library, I'm pretty sure the issue I'm seeing is related to testing-library/react-testing-library#281, which appears to be an issue w/ React 16.8.0 itself, or at least an issue w/ documentation on how to write a test for an async effect that updates state.

[tomwayson]

I looked into the fixes suggested in facebook/react#14769 (comment), but couldn't quite get it working, so will wait for more official guidance.