react-native icon indicating copy to clipboard operation
react-native copied to clipboard

Returning eventSubscription.remove as useEffect cleanup crashes with "undefined is not an object (evaluating emitter)"

Open AlanSl opened this issue 3 years ago • 3 comments
trafficstars

Description

This might be expected behaviour or not worth fixing since there's an easy workaround, but it was tricky to debug and I think it's new behaviour in a recent release (I'm sure I've seen this pattern used since around 0.65 without errors), so I think it's worth having an issue people can find by search.

Confirmed in Android and iOS (and not in react-native-web).


If the remove method of an event subscription is returned as the cleanup function for a useEffect, on dismount, the removal crashes internally with either:

undefined is not an object (reading 'emitter')

Call Stack: EmitterSubscription#remove

...or...

undefined is not an object (evaluating this.emitter)

Doing something like this crashes on dismount, unable to find the subscription's emitter property:

  useEffect(() => {
    const subscription = Dimensions.addEventListener('change', () => {})
    return subscription.remove // crashes, `subscription.emitter` is lost before `remove` is called
  }, [])

Wrapping it in an outer function doesn't crash and works as expected, presumably because trapping the whole subscription object in the return function's scope stops its emitter property from getting deleted?

  useEffect(() => {
    const subscription = Dimensions.addEventListener('change', () => {})
    return () => subscription.remove() // doesn't crash, `subscription.emitter` is preserved
  }, [])

Version

0.69.4

Output of npx react-native info

System:
    OS: macOS 11.6.8
    CPU: (8) x64 Intel(R) Core(TM) i7-1068NG7 CPU @ 2.30GHz
    Memory: 7.56 GB / 32.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.16.0 - ~/.nvm/versions/node/v16.16.0/bin/node
    Yarn: Not Found
    npm: 8.17.0 - ~/.nvm/versions/node/v16.16.0/bin/npm
    Watchman: 2021.06.07.00 - /usr/local/bin/watchman
  Managers:
    CocoaPods: 1.10.1 - /usr/local/bin/pod
  SDKs:
    iOS SDK:
      Platforms: DriverKit 21.2, iOS 15.2, macOS 12.1, tvOS 15.2, watchOS 8.3
    Android SDK:
      API Levels: 29, 30
      Build Tools: 29.0.2, 30.0.3
      System Images: android-29 | Intel x86 Atom_64, android-30 | Google APIs Intel x86 Atom
      Android NDK: Not Found
  IDEs:
    Android Studio: Not Found
    Xcode: 13.2.1/13C100 - /usr/bin/xcodebuild
  Languages:
    Java: 1.8.0_282 - /usr/bin/javac
  npmPackages:
    @react-native-community/cli: Not Found
    react: ~18.0.0 => 18.0.0 
    react-native: 0.69.4 => 0.69.4 
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

Steps to reproduce

  1. Call Dimensions.addEventListener or AccessibilityInfo.addEventListener or similar in a useEffect
  2. Capture the subscription in a variable
  3. Return the subscription's remove method as the useEffect's cleanup function

Snack, code example, screenshot, or link to a repository

https://snack.expo.dev/@alansl/amused-peanut

AlanSl avatar Aug 26 '22 11:08 AlanSl

Hello @AlanSl , I was saw this issue and reproduce in my device. I search about dimensions and see the react native documentation ( https://reactnative.dev/docs/dimensions#example ). In this example they return a function to execute subscription.remove() its same u make in second way.

So I thinking, maybe is because remove is a function and not a object.

So I try to use in first useEffect to execute this code:

  useEffect(() => {
    const listener = Dimensions.addEventListener('change', ({ window }) => setDimensions(window))
    // This crashes because listener.emitter is undefined when listener.remove() tries to access it
    return listener.remove() // return a function
  }, [time1])

when i use listener.remove() the app doesn't crashes

Can u try this way?

I'm early in reactnative contributors, so if I spoke wrong, sorry :+1:

pablolucas890 avatar Aug 26 '22 13:08 pablolucas890

@pablolucas890 Good catch that there is an example in the docs that uses the wrapper function approach (return () => subscription?.remove();).


Your return listener.remove() example is missing the wrapper function: it's creating the subscription then immediately removing it, so it doesn't crash, but it won't work either (if you changed both useEffects in my example to be like this, the "width" text won't update when you rotate the device, because both subscriptions are cancelled immediately after being created).

It should be return () => listener.remove() instead of return listener.remove(), wrapping it in a function so that instead of calling remove right away, React keeps the function and calls it on dismount (see the docs for cleanup functions in useEffect)


The point of this issue is that in most cases, return someFunction (returning the function variable without calling it) and return () => someFunction() would be equivalent if the function is being called without arguments, but in this case (presumably because of some internal detail relying on this or something similar), the first crashes with a non-obvious error, while the second works.

So, this issue isn't a big deal, because the workaround is really simple (do return () => subscription.remove() instead of return subscription.remove), but it's a bit of a "gotcha", so I thought it's worth at least having an issue posted that someone affected can find.

AlanSl avatar Aug 26 '22 14:08 AlanSl

I see, i didn't know listener.remove and () => listener.remove() its same. Thank you!

maybe this problem appears in others examples, u try just with dimensions right?

Good idea to list this problem, maybe peoples trying to use the other way listener.remove and lock in this error

pablolucas890 avatar Aug 26 '22 14:08 pablolucas890

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] avatar Feb 22 '23 18:02 github-actions[bot]

The example Snack above crashes in Expo 46 but doesn't crash when updated to Expo 47 (tested on both Android and iOS), so it looks like this got fixed some time between React Native 0.69.5 and 0.70.5.

AlanSl avatar Feb 24 '23 11:02 AlanSl