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

Use fork instead of spawn for AppWorker #760

Merged
merged 3 commits into from
Jul 31, 2018
Merged

Conversation

itoys
Copy link
Contributor

@itoys itoys commented Jul 31, 2018

Fixes #758

@itoys itoys requested a review from ruslan-bikkinin July 31, 2018 13:08
@itoys
Copy link
Contributor Author

itoys commented Jul 31, 2018

When we use spawn and send message from subprocess with process.send({workerLoaded:true}) we get following error:

SyntaxError: Unexpected token L in JSON at position 0
    at JSON.parse (<anonymous>)
    at Pipe.channel.onread (internal/child_process.js:471:28)

While debugging JSON.parse I saw that it got following message Loaded:true})
I believe that this issue related to nodejs/node#21671
Because of when we use spawn from file running in electron, spawn use system node, instead of fork which use node from electron

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@46e2461). Click here to learn what that means.
The diff coverage is 66.66%.

@@            Coverage Diff            @@
##             master     #760   +/-   ##
=========================================
  Coverage          ?   14.39%           
=========================================
  Files             ?       63           
  Lines             ?     3259           
  Branches          ?      416           
=========================================
  Hits              ?      469           
  Misses            ?     2790           
  Partials          ?        0
Impacted Files Coverage Δ
src/debugger/nodeDebugWrapper.ts 2.91% <0%> (ø)
src/debugger/forkedAppWorker.ts 86.56% <100%> (ø)

@itoys itoys merged commit 2df05fa into master Jul 31, 2018
@itoys itoys deleted the fix/ipc_channel_trim_node_10 branch July 31, 2018 14:01
ruslan-bikkinin added a commit to ruslan-bikkinin/vscode-react-native that referenced this pull request Aug 28, 2018
ruslan-bikkinin added a commit that referenced this pull request Aug 28, 2018
* Revert "FIx incorrect runArguments assignment (#779)"

This reverts commit b9fbf69.

* Revert "Update changelog (#775)"

This reverts commit 5517641.

* Revert "Remove code push images (#771)"

This reverts commit b34bce9.

* Revert "Added support for Build Multiple Versions of React Native app (#766)"

This reverts commit 3e313f6.

* Revert "Update debugging.md (#765)"

This reverts commit fcfec28.

* Revert "fix typo (#763)"

This reverts commit 4dd439f.

* merge

* Revert "Use fork instead of spawn for AppWorker (#760)"

This reverts commit 2df05fa.

* Revert "Added some logging for debug (#759)"

This reverts commit 46e2461.

* Revert "[Android] Add reading SDK location from local.properties (#754)"

This reverts commit 43e1a99.

* Revert "Fixed error while stop expo packager (#753)"

This reverts commit 77b86bf.
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.

3 participants