Use document.currentScript for deriving socket connection url

[dburrows]

When loading a create-react-app bundle from another domain everything works ok except the auto-page reload. The socket connection fails because webpackHotDevClient uses window.location to derive the socket connection url. This commit uses document.currentScript (if available) to derive the SockJS connection url details.

There's a load of ways to find the current executing script in browsers that don't support document.currentScript but they all seemed a bit fragile, so I've made a pragmatic decision to just use currentScript - main downside is that this won't work in IE but that's ok for a dev tool I think. Full support details outlined here: http://caniuse.com/#feat=document-currentscript

I've tested this by loading a normally created app and then loading the same app bundle from another domain, both worked.

P.S. Feel free to close this if you don't think it's a good idea, I'd implemented it anyway and it's quicker to open a PR with it than explain what I wanted to do ahead of time.

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions.

[Timer]

Hey, thanks for the PR!
Does #1301 or #1588 solve this? Just curious.

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[dburrows]

I think #1588 is a good idea, but this PR could be implemented on top of that and still be helpful, HOST/PORT would always override it.

Looking at the original issue #853, I think my fix would fix that issue as long as you didn't expect it to work in IE!

[Timer]

I'm not sure if I understand how this fixes the underlying issue, afaik the browser doesn't know a connection is being proxied.

[dburrows]

The browser doesn't need to know, it just needs to make a socket connection to the same domain that the bundle is loaded from, probably easier with an example:

Say create-react-app is running on localhost:3000 and generating a bundle at localhost:3000/static/js/bundle.js.

We then have a dev server running on localhost:3001 that serves our html (with bootstrap data etc.) and api, and the initial page loads the bundle from localhost:3000/static/js/bundle.js.

Current behaviour: webpackHotDevClient uses the document.location e.g. localhost:3001 to try to connect to the hot dev websocket server when the websocket server is actually running on localhost:3000.

New behaviour: webpackHotDevClient uses document.currentScript.src to work out the websocket url and, as webpackHotDevClient is part of the bundle loaded from 'localhost:3000', it correctly connects to the socket on localhost:3000

[sktt]

@dburrows I don't think this works fully unless your publicPath matches the path of the of hotUpdateChunk. If it does not match, the fallback is to hardreload the page. Try to for example edit one of your stylesheets and verify that the update is applied without a reload.

have look at https://github.com/webpack/webpack/blob/54aa3cd0d6167943713491fd5e1110b777336be6/lib/JsonpMainTemplate.runtime.js#L27

This was what was blocking me from opening a pr like this:
Serving the app from an origin that is different from the devServer's will also likely break publicPath which is normally /. This would also need to be set to origin of document.currentScript.src ie the devServer.. While publicPath can be set in runtime, it's usually expected to be configured through the config file.

Edit: It could also be possible to set publicPath programatically from scripts/start.js, this is where we determine what port to use for the devServer. but it would mean modifying the config files of the devServer and the webpack config from start.js which feels a little bit outside of its scope......

[dburrows]

@sktt ok, I guess a reload is better than nothing though?

[sktt]

It is better with nothing than a half implemented feature

edit: sorry about that. I mean, it would be really nice to have it fully working.. This wouldnt though break anything so ... you might actually be right

[gaearon]

I don't think we'll merge a half-working solution because it will be harder for people to figure out why it doesn't work. Whereas in the current case it's pretty clear we just don't support this use case.

Thanks for PR though!