Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default Android configuration fails out of the box #41

Closed
DaveWelling opened this issue Jun 4, 2021 · 7 comments
Closed

Default Android configuration fails out of the box #41

DaveWelling opened this issue Jun 4, 2021 · 7 comments

Comments

@DaveWelling
Copy link
Contributor

DaveWelling commented Jun 4, 2021

Environment

Tried on both a Pixel 3 XL and a Zebra TC52 (android) device.

"dependencies": {
"react": "17.0.1",
"react-native": "0.64.2"
},
"devDependencies": {
"@babel/core": "^7.12.9",
"@babel/runtime": "^7.12.5",
"@callstack/nativepack": "^1.4.2",
"@react-native-community/eslint-config": "^2.0.0",
"babel-jest": "^26.6.3",
"babel-loader": "^8.2.2",
"eslint": "7.14.0",
"jest": "^26.6.3",
"metro-react-native-babel-preset": "^0.64.0",
"react-test-renderer": "17.0.1",
"terser-webpack-plugin": "^5.1.3",
"webpack": "^5.38.1"
},

node version 15.3.0

Description

If I configure a new project for Windows/Android as described here: https://reactnative.dev/docs/environment-setup

Then make the changes described here:
https://github.com/callstack/nativepack#installation--setup

I paste in the webpack.config.js template from here:
https://github.com/callstack/nativepack/blob/main/templates/webpack.config.js

I can run the bundling server normally and create a bundle without any errors. However, when the app starts, I will receive the following error:

06-04 19:06:02.565  8033 10134 E unknown:ReactNative: Exception in native call from JS
06-04 19:06:02.565  8033 10134 E unknown:ReactNative: com.facebook.react.devsupport.JSException: Unexpected token '='. Expected an opening '(' before a method's parameter list.
06-04 19:06:02.565  8033 10134 E unknown:ReactNative:   at com.facebook.react.bridge.queue.NativeRunnable.run(Native Method)
06-04 19:06:02.565  8033 10134 E unknown:ReactNative:   at android.os.Handler.handleCallback(Handler.java:938)
06-04 19:06:02.565  8033 10134 E unknown:ReactNative:   at android.os.Handler.dispatchMessage(Handler.java:99)
06-04 19:06:02.565  8033 10134 E unknown:ReactNative:   at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:27)
06-04 19:06:02.565  8033 10134 E unknown:ReactNative:   at android.os.Looper.loop(Looper.java:223)
06-04 19:06:02.565  8033 10134 E unknown:ReactNative:   at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:226)
06-04 19:06:02.565  8033 10134 E unknown:ReactNative:   at java.lang.Thread.run(Thread.java:923)
06-04 19:06:02.565  8033 10134 E unknown:ReactNative: Caused by: com.facebook.jni.CppException: Unexpected token '='. Expected an opening '(' before a method's parameter list.
06-04 19:06:02.565  8033 10134 E unknown:ReactNative:
06-04 19:06:02.565  8033 10134 E unknown:ReactNative: no stack

If I then put the app into debug mode using the regular devtools menu, it will load fine. I believe this means that the react-native javascript engine is choking on something that Chrome thinks is fine?

Does this mean the babel loader configuration on the default template webpack.config.js is missing some important library? I don't know how to run something like that down or I would try for you.

If I switch to the Hermes engine, the crash is fatal, killing the app and giving no useful (to me) information in the log.

I can run the same app in metro with no problems.

I went through this exercise because the other app I have been developing suddenly developed a fatal (Hermes) crash for no obvious reason. Turns out it was because I turned the devtools debug mode off . In other words, this same exact error. If I change that app from hermes to the default engine, I get the exact error and stack above.

Reproducible Demo

https://github.com/DaveWelling/reproNativePack.git

It looks like you can reproduce this error simply by following the default android configuration instructions (as I did in the repro above) and making sure your devtools debug mode is turned off on your device.

I also ran the repro above on a completely different windows machine this morning and reproduced the problem. I would try it on a Mac or Ubuntu, but I don't have one here at my house.

@zamotany
Copy link
Contributor

zamotany commented Jun 7, 2021

If you're on Windows then this regex is not matching: https://github.com/callstack/nativepack/blob/main/templates/webpack.config.js#L183
Can you try replacing it with /node_modules(.*[/\\])+@callstack[/\\]nativepack/ and see if that helps?

@zamotany zamotany added the bug label Jun 7, 2021
@DaveWelling
Copy link
Contributor Author

DaveWelling commented Jun 7, 2021

That was it. Both repos I have are working with that change. I wish I had seen it myself, but I am horrible with regex. Thank you!

Would you like a PR or is it easy enough to slip in that change?

@zamotany
Copy link
Contributor

zamotany commented Jun 9, 2021

Yeah, if you can I would greatly appreciate a PR with the fix.

@DaveWelling
Copy link
Contributor Author

DaveWelling commented Jun 10, 2021

@zamotany I tried to do this for you -- you would think it would be easy, but there are quite a few problems with this development code running on a windows OS.
All your test snapshots are written to only work with *nix operating systems. I cannot run your tests locally without significant changes.
I can fix the eslint problems with OS specific line endings and I can fix the failing dependency on nanomatch by adding it as a dev dependency. I can even wrap the explicit checks for OS specific paths with an OS aware replace utiltity function, but I had a strong feeling you would not want me to fiddle with all your test snapshots.

I guess I can just make the one line change for you and submit the PR without checking any of this stuff if you like?

@zamotany
Copy link
Contributor

Don't worry about tests. The change is in the template and we don't have any way of testing it right now. So feel free to just make the change and open a PR. I'll handle the rest.

@DaveWelling
Copy link
Contributor Author

You got it (and thanks again): #42

@zamotany
Copy link
Contributor

The PR is merged, so closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants