setState above TransformWrapper causes cleanup of mousedown

[lesjames]

Describe the bug With version 3.1.1 any setState in the wrapping scope of TransformWrapper will cause cleanupWindowEvents to fire which removes the mousedown event. The result is the inability to pan content with the mouse. Touch events still work.

To Reproduce This example will disable the ability to pan content with the mouse.

() => {
    const [_, setTest] = useState()

    useEffect(
        () => {
            setTimeout(() => setTest('foo'))
        },
        [])

    return (
        <TransformWrapper>>
            <TransformComponent>
                {/* Some content to pan and zoom */}
            </TransformComponent>
        </TransformWrapper>
    )
}

Expected behavior Any React re-render of the wrapping component should not cause event handlers to disappear

[lesjames]

I'm taking a leap here because I don't know this codebase or the decisions that led to this but it seems like instead of doing this...

useEffect(() => {
  instance.update(props);
  return () => {
    instance.cleanupWindowEvents();
  };
}, [instance, props]);

Cleanup should be in it's own useEffect...

useEffect(() => {
  instance.update(props);
}, [instance, props]);

useEffect(() => {
  return () => {
    instance.cleanupWindowEvents();
  };
}, []);

[rjwats]

This is affecting me also, I haven't found a workaround yet.

[adamedmunds]

I had the same issue when using version 3.2.0, I resorted to downgrading to version 3.1.0 and panning seems to be working again.

[2manyvcos]

Some comments to this issue (for completeness):

Incomplete cleanup

It may be worth noting that the performance fix in 3.1.1 does cleanup mouse and key events, but does not cleanup wheel, double-click and touch events.

StrictMode compatibility

Cleanup should be in it's own useEffect...

useEffect(() => {
  instance.update(props);
}, [instance, props]);

useEffect(() => {
  return () => {
    instance.cleanupWindowEvents();
  };
}, []);

The recommendation from @lesjames fixes the issue in most use cases, but is not a good solution for the future, because it does not work with React StrictMode.

See more details on why that matters. (section "Ensuring reusable state")

Instead, it would be better if the event listeners would be added in the same effect that also cleans them up. Also, instance should be in the effect's dependency array for the same reasons:

useEffect(() => {
  instance.initializeWindowEvents();
  instance.handleInitializeWrapperEvents();

  return () => {
    instance.cleanupWindowEvents();
    // TODO: cleanup rest
  };
}, [instance]);

[zzswang]

Created a Minimal playground before I found this issue.

https://codesandbox.io/s/vigilant-mendeleev-9v7tf4?file=/src/App.js

And a workaround is here if anyone need it:

put following code before <TransformWrapper>

  useEffect(() => {
    setTimeout(() => setImageSrc(0), 300);
  }, [imageSrc]);
  
  return (
           <TransformWrapper key={imageSrc}>
              <TransformComponent>
                <ImagePreview src={imageSrc} />
              </TransformComponent>
            </TransformWrapper>
  )

[malthew]

I'm having the same issue and the workaround from @zzswang does not fix it and I imagine it's not a great workaround either for those who have multiple components on one page with multiple useEffects/useStates in each component.

[2manyvcos]

@malthew

The following should work if you are not using StrictMode:

const Demo = React.memo(({ imageSrc }) => {
  return (
    <TransformWrapper key={imageSrc}>
      <TransformComponent>
        <ImagePreview src={imageSrc} />
      </TransformComponent>
    </TransformWrapper>
  );
});

// <Demo imageSrc={imageSrc} />

[malthew]

@malthew

The following should work if you are not using StrictMode:

const Demo = React.memo(({ imageSrc }) => {
  return (
    <TransformWrapper key={imageSrc}>
      <TransformComponent>
        <ImagePreview src={imageSrc} />
      </TransformComponent>
    </TransformWrapper>
  );
});

// <Demo imageSrc={imageSrc} />

You're right, that did fix it. Totally forgot that React had that feature.

Should probably mention though that according to the React docs, it's not recommended that we rely on React.memo if the code doesn't work without it, as it's only meant to be used as performance optimization.

[zzswang]

@malthew

The rule is to avoid multi rerender happened in a really short time.

[piciuok]

I have same issue. Dynamic content is injected to my image viewer as overlay, and interaction with image hangs.