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

Setting the host via the command line doesn't reflect in the snippet code nor the subsequently embedded script #1128

Closed
8 tasks
rmisio opened this issue Jun 23, 2016 · 6 comments

Comments

@rmisio
Copy link

rmisio commented Jun 23, 2016

Issue details

When I set the --host option via the command line, it does adjust the host for the External UI:

image

But, it doesn't affect the host used by the snippet:

image

Even if I manually adjust the hostname in the snippet, the file it fetches still uses location.hostname rather than my passed in host, resulting in XHR errors (and BS not working).

image

The reason I would like to have the host be configurable is because I'm using BS in an Electron app (and hoping to do so via the CLI via an NPM script). Inside of an electron app location.hostname is empty.

To work around the issue, I'm taking the uber-hacky approach of fetching the script with AJAX, modifying the hostname within the script's contents and then embedding the script in the DOM:

image

I'd rather avoid this hackaroo, if possible. If there's another way to accomplish what I need via the CLI, I'd be happy to give it a try.

Steps to reproduce/test case

In an Electron app (or an environment where location.hostname is empty):

  1. browser-sync start --host 'my-host' -f '.js,.html,.tmp/styles/*.css
  2. Notice the Snippet that is output in your console is pulling the host from location rather than your custom host.
  3. Manually modify the src attribute of Snippet so that localhost is used.
  4. Embed the snippet in your HTML file.
  5. Notice that the resulting script causes a bunch of XHR errors because the hostname is empty.

Please specify which version of Browsersync, node and npm you're running

  • Browsersync [ 2.13.0 ]
  • Node [ 6.0.0 ]
  • Npm [ 3.9.5 ]

Affected platforms

  • linux
  • windows
  • [X ] OS X
  • freebsd
  • solaris
  • other (please specify which)

Browsersync use-case

  • API
  • Gulp
  • Grunt
  • [X ] CLI

If CLI, please paste the entire command below

browser-sync start --host 'my-host' -f '*.js,*.html,.tmp/styles/*.css'
@shakyShane
Copy link
Contributor

Let me first clarify (for my own sanity) how the different domains are used within Browsersync.

  1. The Access URL - something like http://localhost:3000 is the address of the HTTP server that Browsersync starts that will be serving: the client script from memory, static files from disk (if using server mode) or proxying requests to your proxy target.

The snippet given on the command line, or that is injected dynamically into page contains something along the lines of...

<script id="__bs_script__">//<![CDATA[
    document.write(
  "<script async src='http://HOST:3000/browser-sync/browser-sync-client.2.13.0.js'><\/script>".replace("HOST", location.hostname));
//]]></script>

where HOST will be overwritten at runtime by the browsers location.hostname value.

The idea here is that locally you'll end up serving http://localhost:3000/browser-sync/browser-sync-client.2.13.0.js, but if you access 192.168.0.1:3000 over your wifi connection, that will still work as the HTTP server is listening to 0.0.0.0 on the host and the request will be for http://192.168.0.1:3000/browser-sync/browser-sync-client.2.13.0.js.

  1. Inside the client script, a similar technique is used to establish the websocket connection. The following line is an example of how it would look in the client-side javascript
bs.socket = bs.io('http://' + location.hostname + ':3000/browser-sync', bs.socketConfig);

This technique allows Browsersync to remain dynamic & work in 90% of use-cases, but here's my take on what your issue is:


You want to manually specify the host, so that the value you provide will be hard-coded into both the snippet output + the run-time-generated client-side JS file. EG: Using Browsersync to watch files on your machine, and then still auto-reload (even when something like location.hostname is absent)

For example, providing --host localhost would result in the following snippet

<script id="__bs_script__">//<![CDATA[
    document.write(
  "<script async src='http://localhost:3000/browser-sync/browser-sync-client.2.13.0.js'><\/script>"
//]]></script>

... and within the client js file, there would be

bs.socket = bs.io('http://localthost:3000/browser-sync', bs.socketConfig);

Does this seem correct?

@rmisio
Copy link
Author

rmisio commented Jul 5, 2016

Yes, that's correct.

I'm using browser-sync with an Electron app and, in this case, the hostname is blank (probably because the app is being served from the file system).

I haven't seen any example on the web that uses BS with an Electron app from the BS command line. The examples I've seen use Gulp, which I believe launches the BS server from code and then the gulp file is managing when to instruct the BS server to reload (or stream).

However, our preference is not to use gulp and to manage our task automation via npm scripts, hence I'm at this roadblock.

FWIW, having the snippet be updated to a cl specified host is a very minor thing, since it's easy for us to update the snippet. The bigger thing is having the client js use a cl specified host (if provided. if not, it could totally default to location.hostname). To get around the latter requires us to explicitly modify that script file, which is something we'd rather avoid (e.g, what if end up wanting to upgrade BS?).

@shakyShane
Copy link
Contributor

With some tweaks, I have this working with electron.

1 problem is that we cannot change host functionality (whether or not it was broken in the past is arguable, but BS is too large now to change how it works).

So, a low-level approach is the following:

browser-sync start --socket.domain="http://localhost:{port}" --script.domain="http://localhost:{port}" -f 'app/'

Which I would happily wrap in a new option name, if someone can think of one.

Note: the above solution, (when released) will give the correct snippet + auto-generated JS and will 'just work' in electron

@shakyShane
Copy link
Contributor

also localhost:{port} will be auto-poulated by Browsersync to be localhost:3000 etc, but that's optional, if you give a full URL, that will be used instead

@shakyShane
Copy link
Contributor

try it out, npm i browsersync/browser-sync <- this will install from github master branch

@shakyShane
Copy link
Contributor

Added the new localOnly flag in 2.14.0 to solve this.

https://github.com/BrowserSync/browser-sync/releases/tag/v2.14.0

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