pmndrs / react-three-fiber

🇨🇭 A React renderer for Three.js
https://docs.pmnd.rs/react-three-fiber
MIT License
27.62k stars 1.6k forks source link

undefined is not an object (evaluating 'screen.orientation.removeEventListener') #3395

Open vaughn-xyz opened 1 week ago

vaughn-xyz commented 1 week ago

Our version in package.json --- 8.17.6

We have reports of user's encountering the above error which seems to stem from use-measure (imported in Canvas). From the source, it would appear as though there are enough logic gates to make sure this wouldn't throw but surprisingly in our build it seems some of these checks are removed.

This is what gets compiled at build time:

// cleanup current scroll-listeners / observers
function removeListeners() {
  if (state.current.scrollContainers) {
    state.current.scrollContainers.forEach(element => element.removeEventListener('scroll', scrollChange, true));
    state.current.scrollContainers = null;
  }

  if (state.current.resizeObserver) {
    state.current.resizeObserver.disconnect();
    state.current.resizeObserver = null;
  }

  if (state.current.orientationHandler) {
    screen.orientation.removeEventListener('orientationchange', state.current.orientationHandler);
  }
}

Yet the source looks like:

// cleanup current scroll-listeners / observers
  function removeListeners() {
    if (state.current.scrollContainers) {
      state.current.scrollContainers.forEach((element) => element.removeEventListener('scroll', scrollChange, true))
      state.current.scrollContainers = null
    }

    if (state.current.resizeObserver) {
      state.current.resizeObserver.disconnect()
      state.current.resizeObserver = null
    }

    if (state.current.orientationHandler) {
      if ('orientation' in screen && 'removeEventListener' in screen.orientation) {
        screen.orientation.removeEventListener('change', state.current.orientationHandler)
      } else if ('onorientationchange' in window) {
        window.removeEventListener('orientationchange', state.current.orientationHandler)
      }
    }
  }

Most specifically, it looks like support is dropped for if ('orientation' in screen && 'removeEventListener' in screen.orientation). While I will investigate our build line to see why it is dropped, this could be a bigger support issue and maybe a different logic gate should be used to ensure it doesn't throw

vaughn-xyz commented 1 week ago

FWIW could an alpha build be done with this change in the source?

/* @preserve */
if ('orientation' in screen && 'removeEventListener' in screen.orientation) {
  screen.orientation.removeEventListener('change', state.current.orientationHandler)
} else if ('onorientationchange' in window) {
  window.removeEventListener('orientationchange', state.current.orientationHandler)
}

Probably need to add /* @preserve */ to both remove and add. I am having difficulties testing if this fixes the problem as I currently cannot reproduce it, only some uses have reported this