Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

[Electron] Remove indirection #498

Merged
merged 4 commits into from
Feb 8, 2017
Merged

Conversation

gaearon
Copy link
Contributor

@gaearon gaearon commented Feb 8, 2017

This vastly simplifies the Electron shell.
Changes:

  • Remove the eval dance and merge backend and embed together
  • Remove the code that was only necessary when RN packager proxied socket connection
  • Backport reconnection logic and memory leak fix from RN

There is no need to eval the backend because we can just import it.
@gaearon gaearon merged commit f4b774e into facebook:master Feb 8, 2017
hasClosed = true;
setTimeout(() => {
connectToDevTools(options);
}, 2000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want to drop this to 500 -- 2 seconds feels like a long time to wait for reconnection

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was intentional change from RN team because they had some issues with 200ms reconnection (it was making things slow or something). 500ms is better than 200ms but I don't want to touch it without understanding the issues they had in the first place. Maybe @yungsters or @javache can provide more details.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool sounds good

@gaearon gaearon deleted the remove-indirection branch February 9, 2017 15:02
pastelsky pushed a commit to pastelsky/react-devtools that referenced this pull request Feb 25, 2017
* Remove the `eval` indirection

There is no need to eval the backend because we can just import it.

* Unify "backend" and "embed" into one file

* Bump ws dep

* Fix lint
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants