facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
119.03k stars 24.31k forks source link

Deeplinking is being interrupted by "Cannot read property 'apply' of undefined". #35577

Closed seco35 closed 7 months ago

seco35 commented 1 year ago

Description

This error occured after upgrading to 0.7x from 0.67 and deeplink worked properly before doing the upgrade.

Have to mention that in both cases (background state & starting through deeplink) the url of the deeplink is there (looking with console.log()). This error is being thrown only if the app is in background, starting the app through deeplink throws no error.

TypeError: Cannot read property 'apply' of undefined, js engine: hermes

While debugging on simulator, I could just minimize the thrown error and actions defined for the deeplink are processed properly. However the thrown error seems to block the execution of the deeplink actions on a real device.

The error is caused as shown in the thrown error at emit()-method of EventEmitter.js (Line 105):

registration.listener.apply(registration.context, args);

Adding a check if the listener is undefined before execution solves the issue for me and deeplink works as expected (both on the simulator and the real device):

if(typeof registration.listener !== "undefined" )
          registration.listener.apply(registration.context, args);

Unfortunately I am not sure, how this does effect the app in general and whether it must be solved by doing something else.

Version

0.70.4

Output of npx react-native info

System: OS: macOS 13.0 CPU: (8) x64 Intel(R) Core(TM) i5-8257U CPU @ 1.40GHz Memory: 32.26 MB / 16.00 GB Shell: 5.8.1 - /bin/zsh Binaries: Node: 16.14.0 - /usr/local/bin/node Yarn: 1.22.19 - /usr/local/bin/yarn npm: 8.3.1 - /usr/local/bin/npm Watchman: 2022.03.21.00 - /usr/local/bin/watchman Managers: CocoaPods: 1.11.3 - /usr/local/bin/pod SDKs: iOS SDK: Platforms: DriverKit 21.4, iOS 16.0, macOS 12.3, tvOS 16.0, watchOS 9.0 Android SDK: API Levels: 30, 31, 32 Build Tools: 30.0.2, 30.0.3, 32.0.0, 32.1.0 System Images: android-30 | Google APIs Intel x86 Atom, android-30 | Google Play Intel x86 Atom, android-31 | Google Play Intel x86 Atom_64 Android NDK: Not Found IDEs: Android Studio: 2021.2 AI-212.5712.43.2112.8609683 Xcode: 14.0.1/14A400 - /usr/bin/xcodebuild Languages: Java: 1.8.0_322 - /usr/bin/javac npmPackages: @react-native-community/cli: Not Found react: 18.1.0 => 18.1.0 react-native: 0.70.4 => 0.70.4 react-native-macos: Not Found npmGlobalPackages: react-native: Not Found

Steps to reproduce

Create a project and configure deeplinking using the docs.

Add to AppDelegate.m:

- (BOOL)application:(UIApplication *)application
   openURL:(NSURL *)url
   options:(NSDictionary<UIApplicationOpenURLOptionsKey,id> *)options
{
  return [RCTLinkingManager application:application openURL:url options:options];
}

- (BOOL)application:(UIApplication *)application continueUserActivity:(nonnull NSUserActivity *)userActivity
 restorationHandler:(nonnull void (^)(NSArray<id<UIUserActivityRestoring>> * _Nullable))restorationHandler
{
 return [RCTLinkingManager application:application
                  continueUserActivity:userActivity
                    restorationHandler:restorationHandler];
}

in JSX:

 Linking.getInitialURL().then(url => { //ACTIONS FOR DEEPLINK START }
 Linking.addEventListener('url',(url)=>{ //ACTIONS FOR DEEPLINK DURING RUNTIME})

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

Bug

s123121 commented 1 year ago

Upgrading from 0.67 to 0.70.6 and meet exactly this problem

taoqf commented 1 year ago

I came to this, too. and I find the null modules is always the multiple depended in my project.

olosegres commented 1 year ago

Same problem after update to 0.70.6

seco35 commented 1 year ago

@olosegres @s123121 @taoqf Have you tried the workaround (adding check for undefined)? Does it solve your issues too?

SerafinAna commented 1 year ago

I also have this issue with React native 0.69.7 does anyone use in the project firebase package. maybe this is causing the problem

seco35 commented 1 year ago

I also have this issue with React native 0.69.7

does anyone use in the project firebase package. maybe this is causing the problem

No firebase usage within my project.

gdeusdara commented 1 year ago

Same problem after updating to 0.71.0

iBotPeaches commented 1 year ago

@seco35 - I investigated this and I believe a regression occurred in this commit - https://github.com/facebook/react-native/commit/e5c5dcd9e26e9443f59864d9763b049e0bda98e7

old

// The subscription may have been removed during this event loop.
if (subscription && subscription.listener) {
    subscription.listener.apply(subscription.context, args);
}

new

if (registrations != null) {
    for (const registration of [...registrations]) {
        registration.listener.apply(registration.context, args);
    }
}

It seems a possibility that we could have a registration without listener. I'm guessing since this was changed in RN70 that it might be at least the original issue reported here. Thought the typing looks like this shouldn't be possible :/

TypeError: Cannot read property 'apply' of undefined, js engine: hermes

@yungsters - Sorry for ping, but you authored the commit I linked and perhaps have an idea if I'm off base or not on this research.

DerBasler commented 1 year ago

Thanks for the investigation @iBotPeaches hopefully they will merge it soon.

AlexandreMuskus commented 1 year ago

hey guys!

when is this correction expected to be published?

yungsters commented 1 year ago

This error is symptomatic of EventEmitter.addListener(…) being called with a non-function second argument.

Can someone help me understand the source of the non-function value being supplied to the 2nd argument to EventEmitter.addListener(…)? That is what should be fixed here.

cc @javache

DerBasler commented 1 year ago

I was able to evaluate the problematic package in my case was: import { TextField } from '@ubaids/react-native-material-textfield'; which I replaced with import { TextField } from 'rn-material-ui-textfield';

But the problem only happened in a specific scenario but I will do some further investigation once I find some time.

**Turns out that only fixed the issue on IOS 🤔 man this is confusing

gdeusdara commented 1 year ago

I put a console.log in the addListener function:

  addListener<TEvent: $Keys<TEventToArgsMap>>(
    eventType: TEvent,
    listener: (...args: $ElementType<TEventToArgsMap, TEvent>) => mixed,
    context: mixed,
  ): EventSubscription {
    console.log('onAddListener - ', eventType, listener) // <-- THIS LINE HERE
    const registrations = allocate(this._registry, eventType);
    const registration: Registration<$ElementType<TEventToArgsMap, TEvent>> = {
      context,
      listener,
      remove(): void {
        registrations.delete(registration);
      },
    };
    registrations.add(registration);
    return registration;
  }

And in the emit function:

emit<TEvent: $Keys<TEventToArgsMap>>(
    eventType: TEvent,
    ...args: $ElementType<TEventToArgsMap, TEvent>
  ): void {
    const registrations: ?Set<
      Registration<$ElementType<TEventToArgsMap, TEvent>>,
    > = this._registry[eventType];
    console.log('inside emit function - ',eventType, args, registrations, this._registry); // <--- THIS LINE HERE
    if (registrations != null) {
      for (const registration of [...registrations]) {
        registration.listener?.apply(registration.context, args); // <-- also put a verification here to be able to initialize app
      }
    }
  }

It turns out, with the debug version, everything seems to be perfectly fine. like this:

Screenshot 2023-02-09 at 11 13 35

But when I build the release version for android, it comes like this:

Screenshot 2023-02-09 at 11 15 20

So any event I call in my app does not receive a response. Such as Network requests, hardwareBackPress, etc.

I've just migrated from react-native 0.61.5 to 0.71.2. I am probably doing something wrong.

javache commented 1 year ago

So any event I call in my app does not receive a response.

This seems different from the original issue, which was causing a crash when events were emitted.

Is it possible you have multiple copies of react-native in your application?

gdeusdara commented 1 year ago

So any event I call in my app does not receive a response.

This seems different from the original issue, which was causing a crash when events were emitted.

Is it possible you have multiple copies of react-native in your application?

This new problem is happening because I used this trick in the first comment:

if(typeof registration.listener !== "undefined" ) registration.listener.apply(registration.context, args);

But, in my case, I just used the ? before .apply

registration.listener?.apply(registration.context, args);

If I don't use this, the same thing described in the original issue happens to me.

TypeError: Cannot read property 'apply' of undefined, js engine: hermes

I was able to make my app work properly by downgrading to version 0.69.8 (which is the last version that doesn't have this new EventEmitter)

yungsters commented 1 year ago

What happens if you change addListener to throw if it receives a non-function second argument?

Do you get an error and more meaningful stack trace?

gdeusdara commented 1 year ago

@yungsters I put here, like this:

addListener<TEvent: $Keys<TEventToArgsMap>>(
    eventType: TEvent,
    listener: (...args: $ElementType<TEventToArgsMap, TEvent>) => mixed,
    context: mixed,
  ): EventSubscription {

    console.log(eventType, listener, typeof listener) // <-- CONSOLE.LOG HERE
    if (typeof listener != 'function') { // <-- VERIFICATION
      throw new Error(
        'No listener',
      );
    }

    const registrations = allocate(this._registry, eventType);
    const registration: Registration<$ElementType<TEventToArgsMap, TEvent>> = {
      context,
      listener,
      remove(): void {
        registrations.delete(registration);
      },
    };
    registrations.add(registration);
    return registration;
  }

And also this:

console.log('inside emit function - ',eventType, args, registrations, this._registry); // <--- THIS LINE HERE

in the emit function, like in my previous comment.

These are the logs when I initialize the release version of the app:

info Starting logkitty
[11:43:32] D | ReactNative ▶︎ [CodePush] Loading JS bundle from "assets://index.android.bundle"

[11:43:33] I | ReactNativeJS ▶︎ 'didUpdateDimensions', [Function], 'function'

[11:43:33] I | ReactNativeJS ▶︎ 'hardwareBackPress', [Function], 'function'

[11:43:33] I | ReactNativeJS ▶︎ 'keyboardDidShow', [Function], 'function'
                             â”” 'keyboardDidHide', [Function], 'function'

[11:43:33] I | ReactNativeJS ▶︎ 'onGestureHandlerEvent', [Function: S], 'function'
                             â”” 'onGestureHandlerStateChange', [Function: S], 'function'

[11:43:33] I | ReactNativeJS ▶︎ 'ReactNativeBlobUtilMessage', [Function], 'function'

[11:43:33] I | ReactNativeJS ▶︎ 'ReactNativeBlobUtilMessage', [Function], 'function'

[11:43:33] I | ReactNativeJS ▶︎ 'websocketMessage', [Function], 'function'
                             │ 'websocketOpen', [Function], 'function'
                             â”” 'websocketClosed', [Function], 'function'

[11:43:33] I | ReactNativeJS ▶︎ 'websocketFailed', [Function], 'function'

[11:43:33] I | ReactNativeJS ▶︎ 'appearanceChanged', [Function], 'function'

[11:43:33] I | ReactNativeJS ▶︎ Running "myApp
                             │ 'collectBugExtraData', [Function: value], 'function'
                             â”” 'collectRedBoxExtraData', [Function: value], 'function'

[11:43:33] I | ReactNativeJS ▶︎ 'inside emit function - ', 'RNDeviceInfo_powerStateDidChange', [ 'charging' ], undefined, { didUpdateDimensions: {},
                             │ hardwareBackPress: {},
                             │ keyboardDidShow: {},
                             │ keyboardDidHide: {},
                             │ onGestureHandlerEvent: {},
                             │ onGestureHandlerStateChange: {},
                             │ ReactNativeBlobUtilMessage: {},
                             │ websocketMessage: {},
                             │ websocketOpen: {},
                             │ websocketClosed: {},
                             │ websocketFailed: {},
                             │ appearanceChanged: {},
                             │ collectBugExtraData: {},
                             â”” collectRedBoxExtraData: {} }

It does not Throw, and the this._registry is without the correct Set(s) inside. Debug works fine.

Maybe is some config in the release mode that is wrong? It does not make sense to me

Naman-Garaaz commented 1 year ago

I am also facing the same issue after upgrading from 066.4 to 0.70.6 reactnative

AlexandreMuskus commented 1 year ago

hey guys!

when is this correction expected to be published?

DerBasler commented 1 year ago

@AlexandreMuskus I just updated the pr. But now it will not fix your issue it will only help you to find the problem. If you need a quick fix I would advise you to make a script which changes the following https://github.com/facebook/react-native/pull/36087/commits/2a2c1f29ecd5a9a8a4ef7bb5c6bc7ab3dc081b1e

teopaquet commented 1 year ago

Hey, When is this correction expected to be published?

ChristopherAChase commented 1 year ago

Hello, just curious if there is any update on this issue or not? I'm running into the exact same error as OP.

yungsters commented 1 year ago

React Native v0.72 will include https://github.com/facebook/react-native/commit/2780ba38ff23f4c5e717b8fd8a733b649701f00c, which changes EventEmitter to throw a TypeError when addListener is called with a second argument that is not a function.

This should help everyone here narrow down the originating source of this error in order to find out where the non-function is coming from (and where to fix the root cause).

mikollo commented 1 year ago

Hi! I encountered the same problem and made a slight adjustment to this line, which seemed a bit off. The spread operation appeared unnecessary:

-      for (const registration of [...registrations]) {
+      for (const registration of registrations) {

To my surprise, this change fixed the issue cc @yungsters . The problem arose after upgrading from version 0.69 to 0.72. I have Hermes enabled.

Here's a complete patch file for anyone else who might be grappling with this issue:

diff --git a/node_modules/react-native/Libraries/vendor/emitter/EventEmitter.js b/node_modules/react-native/Libraries/vendor/emitter/EventEmitter.js
index 6bd2025..6b64e0c 100644
--- a/node_modules/react-native/Libraries/vendor/emitter/EventEmitter.js
+++ b/node_modules/react-native/Libraries/vendor/emitter/EventEmitter.js
@@ -109,7 +109,7 @@ export default class EventEmitter<TEventToArgsMap: {...}>
       Registration<$ElementType<TEventToArgsMap, TEvent>>,
     > = this._registry[eventType];
     if (registrations != null) {
-      for (const registration of [...registrations]) {
+      for (const registration of registrations) {
         registration.listener.apply(registration.context, args);
       }
     }
Ancez commented 1 year ago

I had the same error, and I fixed it by doing this image

For me, the incorrect setup of expo-notifications caused the error.

I hope this helps some else out there

Edit I agree with @mikollo's comment about changing [...registrations] to registrations

javache commented 1 year ago

Hi! I encountered the same problem and made a slight adjustment to this line, which seemed a bit off. The spread operation appeared unnecessary:

-      for (const registration of [...registrations]) {
+      for (const registration of registrations) {

To my surprise, this change fixed the issue cc @yungsters . The problem arose after upgrading from version 0.69 to 0.72. I have Hermes enabled.

The spread is required to allow listeners to be added and removed during the execution of the event callback.

mikollo commented 1 year ago

Hi! I encountered the same problem and made a slight adjustment to this line, which seemed a bit off. The spread operation appeared unnecessary:

-      for (const registration of [...registrations]) {
+      for (const registration of registrations) {

To my surprise, this change fixed the issue cc @yungsters . The problem arose after upgrading from version 0.69 to 0.72. I have Hermes enabled.

The spread is required to allow listeners to be added and removed during the execution of the event callback.

So the idea is to make a copy of registrations? @javache But, since it is a Set, it should've been done without the conversion to an array, which is unnecessary, like this:

      for (const registration of new Set(registrations)) {

updated patch:

diff --git a/node_modules/react-native/Libraries/vendor/emitter/EventEmitter.js b/node_modules/react-native/Libraries/vendor/emitter/EventEmitter.js
index 6bd2025..ab63eff 100644
--- a/node_modules/react-native/Libraries/vendor/emitter/EventEmitter.js
+++ b/node_modules/react-native/Libraries/vendor/emitter/EventEmitter.js
@@ -109,7 +109,7 @@ export default class EventEmitter<TEventToArgsMap: {...}>
       Registration<$ElementType<TEventToArgsMap, TEvent>>,
     > = this._registry[eventType];
     if (registrations != null) {
-      for (const registration of [...registrations]) {
+      for (const registration of new Set(registrations)) {
         registration.listener.apply(registration.context, args);
       }
     }

It solves the issue, which seems to be caused by a bug in the JS engine/compiler/transpiler (?).

Essentially, it incorrectly converts a Set to an Array when using the spread operator, which can be reproduced like this:

         const registrations = new Set();

          registrations.add(1);

          registrations.add(2);

          registrations.add(3);

          console.log([...registrations].join(" ")); // logs [object Set], while it should be 1 2 3

          console.log(Array.from(registrations).join(" ")); // 1 2 3

I cannot reproduce it on https://hermesengine.dev/playground/, so I presume it is caused by Babel.

javache commented 1 year ago

It solves the issue, which seems to be caused by a bug in the JS engine/compiler/transpiler (?).

That's great! It would be good to understand which layer of the stack is causing this bug though. Polyfilling of Set/Map should no longer be required for RN builds, since modern JS engines support them natively.

yungsters commented 1 year ago

@mikollo — It seems like the spread operator (...) is not correctly iterating over Set in your application.

Can you try verifying the following in your environment?

console.log([...new Set([1, 2, 3])].length);
// Should be 3, but seems to be 1 for you.

console.log([...new Set([1, 2, 3])].toString());
// Should be "1,2,3", but seems to be "[object Set]" for you.

Are you able to reproduce this in a standard application setup?

We can certainly (and probably should) replace [...registrations] with new Set(registrations) or Array.from(registrations), but if there is something else that's broken with spreading iterators into arrays… that's also worrisome and worth fixing.

yungsters commented 1 year ago

In the meantime, I've created https://github.com/facebook/react-native/pull/39525 to change EventEmitter to use Array.from instead of the spread operator.

mikollo commented 1 year ago

I now see what was the root cause, I had the loose option turned on in my Babel config:

    [
      "@babel/plugin-transform-spread",
      {
        loose: true,
      },
    ],

and it transpiles [...registrations] to [].concat(registrations). And since concat does not treat all array-like objects as arrays by default, the problem arises. It was actually predicted in this comment for the Babel PR which introduced the concat mechanism - https://github.com/babel/babel/pull/9108#issuecomment-448201060.

I believe this is not a standard application setup (loose mode turned on), and therefore a bug on the react-native side. I guess this issue can be closed. Thank you for your help!

yungsters commented 1 year ago

Thanks for following up with that context, @mikollo!

I am curious whether other folks who have encountered this issue (and commented above) also made use of the loose option when configuring @babel/plugin-transform-spread. It would be great if anyone else who has reported this issue could chime in on whether their problems are the result of the same root cause.

github-actions[bot] commented 7 months ago

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] commented 7 months ago

This issue was closed because it has been stalled for 7 days with no activity.