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

Unnecessary inner server when using browser-run #28

Closed
3cp opened this issue Jun 7, 2019 · 3 comments · Fixed by #29
Closed

Unnecessary inner server when using browser-run #28

3cp opened this issue Jun 7, 2019 · 3 comments · Fixed by #29

Comments

@3cp
Copy link
Contributor

3cp commented Jun 7, 2019

I got some strange lock-up in windows when running browser-run --static . with all latest npm dependencies.

Not sure exactly which dependency caused the window issue. But when I dig deeper, it seems the inner http server in electron-stream is unnecessary when browser-run calls this module.

I was able to somehow bypass the windows issue by force electron-stream to reuse the http server created by browser-run.

Electron.prototype._onfinish = function(){
  var self = this;
  var cb = this._spawn.bind(this)
  if (this.killed) return;
  this.source.push(null);

  // When browser-run runs this,
  // use the server booted by browser-run,
  // instead of creating new one in _createSourceUrl.
  if (this.opts.loc) {
    return cb(this.opts.loc);
  }

  if (this.opts.nodeIntegration) {
    return this._createNodeUrl(cb)
  }

  this._createSourceUrl(cb);
};

Let me know if this makes sense.

@juliangruber
Copy link
Owner

Hmm this is weird! I don't have access to a windows machine right now. Would you be interested in owning this and creating PR(s) for it?

@3cp
Copy link
Contributor Author

3cp commented Jun 8, 2019

As I said, I am not sure this is the cause of my Windows issue. But if you think this special support of browser-run makes sense, I can create a PR.

@3cp
Copy link
Contributor Author

3cp commented Jun 9, 2019

Might be ecstatic related http-party/http-server#534

juliangruber pushed a commit that referenced this issue Jun 17, 2019
* fix: avoid creating inner http server for browser-run session

closes #28

* doc: document new option "loc" for using an existing http server
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 a pull request may close this issue.

2 participants