Skip to content

Conversation

@byCedric
Copy link
Member

Fixes #663

@brentvatne
Copy link
Member

thanks!

@brentvatne brentvatne merged commit 441835a into expo:master May 23, 2018
@byCedric byCedric deleted the fix/security-issues branch May 23, 2018 22:45
brentvatne added a commit that referenced this pull request May 24, 2018
@brentvatne
Copy link
Member

It seems that there was some change to xdl that breaks starting the packager here, I had to revert the commit. I'll investigate further maybe next week. You can verify by adding [email protected] to an app and trying to start the packager, it stays on "Starting packager"

@byCedric
Copy link
Member Author

Sure, do you prefer to have this tested on an existing project or a brand new one?

@brentvatne
Copy link
Member

either works

@byCedric
Copy link
Member Author

Ok, starting a new project using npx create-react-native-app . and chaning react-native-scripts to 1.14.1 results in this:

cedric at Cedrics-MacBook-Pro in ~/Projects/bycedric/expo-test                                                                                                                                                                          22:27
> NODE_ENV=development yarn start
yarn run v1.7.0
$ react-native-scripts start
10:28:00 PM: Starting packager...
***ERROR STARTING PACKAGER***
No issue with doctor-watchman-version
No issue with doctor-problem-checking-watchman-version
No issue with doctor-both-app-and-exp-json
No issue with doctor-schema-validation
No issue with doctor-validate-asset-fields
No issue with doctor-schema-validation-exception
No issue with doctor-unversioned
No issue with doctor-no-react-native-in-package-json
No issue with doctor-versions-endpoint-failed
No issue with doctor-invalid-sdk-version
No issue with doctor-node-modules-missing
No issue with doctor-react-native-not-installed
No issue with doctor-npm-not-found
No issue with doctor-could-not-run-npm-ls
No issue with doctor-problem-checking-node-modules
No issue with doctor-node-modules-issues
Scanning folders for symlinks in /Users/cedric/Projects/bycedric/expo-test/node_modules (33ms)

The "best part" of this is that the ERROR STARTING PACKAGER is caused by my node version (10) which permanently logs (node:12531) ExperimentalWarning: The fs.promises API is experimental. I think this is resolved when parsing the logs of the process here.

When I wait for some time, my firewall is actually prompting me for node access. So I think it actually starts something. I also put console.log like everywhere in the xdl-src/Project.js#startAsync to see where it stalled. I found out that, down the line in the rns-src/util/packager.js, the success callback actually gets triggered. Which should mean that it started successfully right?

When going even further I noticed that the actual output of the packers are parsed using the output stream of with over here, rns-src/util/packager.js. I logged this too, and found out that I never receive a message with chunk.tag === 'device'. Instead I get tons of messages with tags from expo and metro.

@byCedric
Copy link
Member Author

byCedric commented May 24, 2018

Well, when I was typing this I was debugging even more and found something genius. While I'm not precisely sure why no chunk.tag === 'device' is emitted, I found out that the hotkeys actually do work. So try pressing d, it prints the message like it thinks its working perfectly... Even when starting a simulator using 1.14.0, closing the node server and open a 1.14.1 (with the same simulator running), actually loads the app...

@byCedric
Copy link
Member Author

So, to summarise this, I think the "first/opening message" is being blocked by the fact that the chunk tag is not device anymore. Instead I think it's something like metro. The packager and the services are actually starting and working fine.

Sorry for the spam @brentvatne! Hope it helps.
❤️ the guys from @Peakfijn

@brentvatne
Copy link
Member

hey @byCedric! that does help a lot, thank you! we will go ahead and bump xdl when we release the next version of expo (in the next week or two) and make the appropriate changes for logging that you discovered

@byCedric
Copy link
Member Author

We can't wait 😬

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

Successfully merging this pull request may close these issues.

2 participants