Fix console error #4252

Latropos · 2023-03-21T10:22:40Z

Summary

Fix this issue: #4225

Looks like we have been overwriting console and making its properties not configurable. This PR should fix the problem.

Test plan

Tested on example app

tomekzaw · 2023-03-21T10:51:14Z

src/reanimated2/initializers.ts

+    Object.defineProperties(global.console, {
+      debug: { ...props, value: runOnJS(capturableConsole.debug) },
+      log: { ...props, value: capturableConsole.log },
+      warn: { ...props, value: capturableConsole.warn },
+      error: { ...props, value: capturableConsole.error },
+      info: { ...props, value: capturableConsole.info },
+    });


Do we need this change? I thought it worked without those.

We don't need it, but it lets us get rid of @ts-ignore

I'm not familiar with react-native-reanimated code, but what is the purpose of this Object.defineProperties(...) statement and why only debug is wrapped in runOnJS and the other ones not?

I mean someone in that issue mentioned that they commented out the global.console assignment. Should they expect that something will not work without it? I just downgraded back to v2 instead for now until this is fixed.

We don't need it, but it lets us get rid of @ts-ignore

I think the problem with TypeScript is rather related to the fact that we don't implement all console.* methods but only a subset of them.

why only debug is wrapped in runOnJS and the other ones not?

+1 to this question

Is it really expected to remove all : runOnJS except on debug ?

In the current implementation all console.* methods are wrapped by runOnJS, I don't think we want to remove it now.
https://github.com/software-mansion/react-native-reanimated/blob/main/src/reanimated2/initializers.ts#L172-L176

debug: runOnJS(capturableConsole.debug), log: runOnJS(capturableConsole.log), warn: runOnJS(capturableConsole.warn), error: runOnJS(capturableConsole.error), info: runOnJS(capturableConsole.info),

Yes, we don't want to remove any runOnJS, it was a typo 😅

freeboub · 2023-03-21T16:40:27Z

Tested locally on my side, I confirm it fixes the crash in debug mode, but I have following crash in release mode:

03-21 19:54:18.192 10816 10816 I HermesVM: JSI rethrowing JS exception: Object.defineProperties() called on non-object
03-21 19:54:18.192 10816 10816 I HermesVM: 
03-21 19:54:18.192 10816 10816 I HermesVM: TypeError: Object.defineProperties() called on non-object
03-21 19:54:18.192 10816 10816 I HermesVM:     at defineProperties (native)
03-21 19:54:18.192 10816 10816 I HermesVM:     at anonymous (JavaScript:1:357)
03-21 19:54:18.192 10816 10816 I HermesVM:     at apply (native)
03-21 19:54:18.192 10816 10816 I HermesVM:     at anonymous (JavaScript:1:64)
03-21 19:54:18.196 10816 10816 D AndroidRuntime: Shutting down VM
03-21 19:54:18.198 10816 10816 E AndroidRuntime: FATAL EXCEPTION: main
03-21 19:54:18.198 10816 10816 E AndroidRuntime: Process: com.test.project, PID: 10816
03-21 19:54:18.198 10816 10816 E AndroidRuntime: com.facebook.jni.CppException: Object.defineProperties() called on non-object
03-21 19:54:18.198 10816 10816 E AndroidRuntime: 
03-21 19:54:18.198 10816 10816 E AndroidRuntime: TypeError: Object.defineProperties() called on non-object
03-21 19:54:18.198 10816 10816 E AndroidRuntime:     at defineProperties (native)
03-21 19:54:18.198 10816 10816 E AndroidRuntime:     at anonymous (JavaScript:1:357)
03-21 19:54:18.198 10816 10816 E AndroidRuntime:     at apply (native)
03-21 19:54:18.198 10816 10816 E AndroidRuntime:     at anonymous (JavaScript:1:64)
03-21 19:54:18.198 10816 10816 E AndroidRuntime: 	at com.swmansion.reanimated.Scheduler.triggerUI(Native Method)
03-21 19:54:18.198 10816 10816 E AndroidRuntime: 	at com.swmansion.reanimated.Scheduler$1.run(Scheduler.java:24)
03-21 19:54:18.198 10816 10816 E AndroidRuntime: 	at com.swmansion.reanimated.Scheduler$2.runGuarded(Scheduler.java:43)
03-21 19:54:18.198 10816 10816 E AndroidRuntime: 	at com.facebook.react.bridge.GuardedRunnable.run(GuardedRunnable.java:30)
03-21 19:54:18.198 10816 10816 E AndroidRuntime: 	at android.os.Handler.handleCallback(Handler.java:789)
03-21 19:54:18.198 10816 10816 E AndroidRuntime: 	at android.os.Handler.dispatchMessage(Handler.java:98)
03-21 19:54:18.198 10816 10816 E AndroidRuntime: 	at android.os.Looper.loop(Looper.java:164)
03-21 19:54:18.198 10816 10816 E AndroidRuntime: 	at android.app.ActivityThread.main(ActivityThread.java:6541)
03-21 19:54:18.198 10816 10816 E AndroidRuntime: 	at java.lang.reflect.Method.invoke(Native Method)
03-21 19:54:18.198 10816 10816 E AndroidRuntime: 	at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:240)
03-21 19:54:18.198 10816 10816 E AndroidRuntime: 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:767)
03-21 19:54:18.238  4705  8053 W ActivityManager:   Force finishing activity

piaskowyk · 2023-03-22T17:34:46Z

@Latropos could you test it in release build also?

Latropos · 2023-03-23T10:05:30Z

@freeboub The crash on release was because of the refactor - we can't call defineProperties on console on release build, since console in not an object on release. Hope it's fixed

TypeError: Object.defineProperties() called on non-object

mlazari · 2023-03-23T10:22:50Z

I can confirm that the "property is not configurable" error does not occur with the latest change in this PR and the release build does not crash in my repro project: https://github.com/mlazari/ReanimatedRepro/tree/fix-pr

mlazari · 2023-03-23T11:31:02Z

It looks like the global.console assignment affects console.trace(): https://github.com/mlazari/ReanimatedRepro/blob/console-trace/App.tsx#L63

If I comment out the global.console assignment:

With the global.console assignment:

piaskowyk · 2023-03-30T11:36:48Z

@mlazari I wasn't able to reproduce this problem with console.trace(). Moreover, we modify just global.console on Reanimated JS Runtime, we don't change the console object form React Runtime.

chakravarthi-bb · 2023-04-12T12:30:24Z

Can we make a release for this patch, please??

tiagomsmagalhaes · 2023-04-15T17:13:00Z

@chakravarthi-bb you can append .patch to this url to generate a patch file. just need afterwards to change the path on the diff to the node_modules directory of the package

altarrok · 2023-06-01T15:49:32Z

+1 on making a release for this patch

mlazari · 2023-06-01T15:55:37Z

@altarrok It was already released in 3.1.0 (latest is 3.2.0 now).

## Summary Fix this issue: software-mansion#4225 Looks like we have been overwriting console and making its properties not configurable. This PR should fix the problem. ## Test plan Tested on example app --------- Co-authored-by: Aleksandra Cynk <aleksandracynk@aleksandras-mbp.home> Co-authored-by: Aleksandra Cynk <aleksandracynk@swmansion.com> Co-authored-by: Krzysztof Piaskowy <krzysztof.piaskowy@swmansion.com>

monicatiba · 2023-07-03T12:33:29Z

Do we have a fix for this? I'm facing the same error even with the last version of the lib and I cannot find a way to fix it. I tried everything. Running on IOS and MacOs

Error log: in js engine: hermes

mlazari · 2023-07-03T15:34:34Z

@monicatiba Are you sure that is related to react-native-reanimated? The stack trace looks different from the one in #4225 and as far as I can tell that is fixed in the latest version.

ZakaryH · 2023-11-02T02:41:50Z

@monicatiba I had this same error and stack trace. for me the problem was that I was applying a useAnimatedStyle style to a View rather than an Animated.View.

Latropos changed the title ~~Fix smooth scroll android~~ Fix console error Mar 21, 2023

Fix smooth scroll android

5b42edd

Latropos force-pushed the @acynk/fix-console-bug branch from 043e56e to 5b42edd Compare March 21, 2023 10:25

Latropos marked this pull request as ready for review March 21, 2023 10:44

tomekzaw reviewed Mar 21, 2023

View reviewed changes

Latropos requested a review from tomekzaw March 21, 2023 11:19

piaskowyk linked an issue Mar 21, 2023 that may be closed by this pull request

"Property not configureable" error on Development when upgrading to V3 #4225

Closed

Aleksandra Cynk added 2 commits March 23, 2023 10:31

Add missing runOnJS

080dcbe

Fix the other error

58a0373

Latropos requested a review from piaskowyk March 23, 2023 10:05

NedStar72 mentioned this pull request Mar 29, 2023

TypeError: property is not configurable react-navigation/react-navigation#11289

Closed

11 tasks

Merge branch 'main' into @acynk/fix-console-bug

86d72ac

piaskowyk approved these changes Mar 30, 2023

View reviewed changes

piaskowyk added this pull request to the merge queue Mar 30, 2023

Merged via the queue into main with commit 901da2c Mar 30, 2023

piaskowyk deleted the @acynk/fix-console-bug branch March 30, 2023 11:37

mikebouwmans mentioned this pull request Mar 31, 2023

TypeError: property is not configurable (in NavigationContainerInner) react-navigation/react-navigation#9627

Closed

jphilipstevens mentioned this pull request Apr 6, 2023

Weird React Native error: property is not configurable styled-components/styled-components#3981

Open

guillaumegaluz mentioned this pull request Jun 8, 2023

Cannot find module '@babel/plugin-proposal-optional-chaining' #4538

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix console error #4252

Fix console error #4252

Latropos commented Mar 21, 2023

tomekzaw Mar 21, 2023

Latropos Mar 21, 2023

mlazari Mar 21, 2023

mlazari Mar 21, 2023

tomekzaw Mar 21, 2023

freeboub Mar 21, 2023

piaskowyk Mar 22, 2023 •

edited

Latropos Mar 23, 2023 •

edited

freeboub commented Mar 21, 2023 •

edited

piaskowyk commented Mar 22, 2023

Latropos commented Mar 23, 2023

mlazari commented Mar 23, 2023

mlazari commented Mar 23, 2023

piaskowyk commented Mar 30, 2023

chakravarthi-bb commented Apr 12, 2023

tiagomsmagalhaes commented Apr 15, 2023

altarrok commented Jun 1, 2023

mlazari commented Jun 1, 2023

monicatiba commented Jul 3, 2023 •

edited

mlazari commented Jul 3, 2023

ZakaryH commented Nov 2, 2023

Fix console error #4252

Fix console error #4252

Conversation

Latropos commented Mar 21, 2023

Summary

Test plan

tomekzaw Mar 21, 2023

Choose a reason for hiding this comment

Latropos Mar 21, 2023

Choose a reason for hiding this comment

mlazari Mar 21, 2023

Choose a reason for hiding this comment

mlazari Mar 21, 2023

Choose a reason for hiding this comment

tomekzaw Mar 21, 2023

Choose a reason for hiding this comment

freeboub Mar 21, 2023

Choose a reason for hiding this comment

piaskowyk Mar 22, 2023 • edited

Choose a reason for hiding this comment

Latropos Mar 23, 2023 • edited

Choose a reason for hiding this comment

freeboub commented Mar 21, 2023 • edited

piaskowyk commented Mar 22, 2023

Latropos commented Mar 23, 2023

mlazari commented Mar 23, 2023

mlazari commented Mar 23, 2023

piaskowyk commented Mar 30, 2023

chakravarthi-bb commented Apr 12, 2023

tiagomsmagalhaes commented Apr 15, 2023

altarrok commented Jun 1, 2023

mlazari commented Jun 1, 2023

monicatiba commented Jul 3, 2023 • edited

mlazari commented Jul 3, 2023

ZakaryH commented Nov 2, 2023

piaskowyk Mar 22, 2023 •

edited

Latropos Mar 23, 2023 •

edited

freeboub commented Mar 21, 2023 •

edited

monicatiba commented Jul 3, 2023 •

edited