Created by: scisci
Adds HOST and PORT env variables to react app so websocket can use them instead of window.location.
This addresses the issue listed here: https://github.com/facebookincubator/create-react-app/issues/853
This can be tested by adding a .env
file to an app with the following content:
HOST=127.0.0.1
PORT=3000
Then run the app by visiting http://localhost:3000. If you look at the network requests, you will see the websocket is using 127.0.0.1 and not localhost.
Activity
Created by: 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.
Created by: scisci
I don't use pow so I can't answer for certain. Maybe @thom-nic could chime in.
But I think it should because create react app would be listening on localhost:3000 for the websocket and that is what would be requested from the react app as long as its specified under host/port env vars.
Currently if its getting served through foo.dev via window.location, the web socket would get denied by the pow server.
Created by: scisci
The issue is if you are running a separate web server as your main backend for a project and are proxying requests to the react app bundle.js through it.
As I said in the issue, I was able to overcome this issue by implementing a websocket proxy on my server, so its not crucial for me anymore, but it seems to make sense. To assume that window.location is the server hosting the websocket seems optimistic. But I see your point about create react app only for simple setups.
Created by: fabiosussetto
This is going to be helpful for me as well as I'm trying to use react-create-app with a Django backend where I need to render the index.html file in the public dir server side (mainly to pass some initial data and settings to React). At the moment I have the problem where the websocket is trying to connect to http://localhost:8000 (Django) instead of http://localhost:3000 (Webpack dev server). I confirm this fix would solve my issue.
Created by: dan-kez
Thanks for making this PR! I ran into an issue with developing chrome extensions where
window.location.protocol
is set tochrome-extension:
causing an error with SockJS.This can be fixed by adding the following:
var connection = new SockJS(url.format({ - protocol: window.location.protocol, + protocol: process.env.REACT_APP_SOCKJS_PROTOCOL || window.location.protocol, ... }));
Edit: Also, loading of static content will also become an issue in dev since
publicPath
inwebpack.config.dev.js
is hardcoded to/
. There may be a different workaround that I'm unaware of.Created by: dan-kez
@sktt That's even better. I also agree that this should probably be done for
publicPath
too.
Edit: After doing some more development around chrome extensions I've realized that the above proposal will not cover all situations. Chrome will not allow external dependencies (e.g.
<script src='https://localhost:3000/foo.js'>
) to access its chrome storage (chrome.storage
). The dependencies that access those APIs need to be loaded via thechrome-extension://
protocol.Given this, I believe publicPath should default to @sktt 's proposal but have the ability to be overridden by another environment variable e.g.
REACT_APP_PUBLIC_PATH
.In the chrome extension use case, a developer needs SockJs to point to the localhost but the injected scripts to source from
chrome-extension://mhleglpkaimohejeochljenlpohnpnph/
(their given app id).I'm happy to clarify further if there are any questions about this use case.
Created by: sktt
Problem about publicPath: webpack dev server uses it both for hot chunks and assets. :s On Mon, 8 May 2017 at 00:33, Daniel Kezerashvili notifications@github.com wrote:
@sktt https://github.com/sktt That's even better. I also agree that this should probably be done for publicPath too.
Edit: After doing some more development around chrome extensions I've realized that the above proposal will not cover all situations. Chrome will not allow external dependencies (e.g.
Created by: sktt
There is also suggestions about configuring this from the script tag such as #1994. However, publicPath depends on the host and port too for assets and hot chunks so with that, configless, approach the publicPath must be set on runtime by also setting webpack_public_path which could be confusing while you'd expect it to be by the webpack config On Mon, 15 May 2017 at 14:38, SciSci notifications@github.com wrote:
@scisci commented on this pull request.
In packages/react-scripts/config/env.js https://github.com/facebookincubator/create-react-app/pull/1588#discussion_r116478834 :
@@ -29,7 +29,12 @@ function getClientEnvironment(publicUrl) { // For example, <img src={process.env.PUBLIC_URL + '/img/logo.png'} />. // This should only be used as an escape hatch. Normally you would put // images into the
src
andimport
them in code to get their paths.-
'PUBLIC_URL': publicUrl
-
'PUBLIC_URL': publicUrl,
-
// Useful for allowing the hot dev websocket to connect to the host
-
// specified in the env file, instead of window.location, which could
-
// be wrong if the dev server is being proxied.
-
'HOST': process.env.HOST || '',
-
'PORT': process.env.PORT || ''
I believe it needs to be configured somehow. The .env file was the only place that I knew of that allowed this, that's why I chose to put it there.
I wasn't aware that variables could be shared in another way but if you point me in the direction of an example I could change this.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/facebookincubator/create-react-app/pull/1588#discussion_r116478834, or mute the thread https://github.com/notifications/unsubscribe-auth/ABXj8Jm38sY2eujoXdv7Dn54_T5vyfwtks5r6EcngaJpZM4MFEXK .
-
Created by: roborourke
Package.json seems like the best place, could mirror the
devServer
section of a webpack config On Fri, 19 May 2017 at 13:53, David Burrows notifications@github.com wrote:As it doesn't seem we're going to do #1994 https://github.com/facebookincubator/create-react-app/pull/1994 how do we get this PR over the line? If we're not using env vars what else are we going to use? Config in package.json?
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/facebookincubator/create-react-app/pull/1588#issuecomment-302683159, or mute the thread https://github.com/notifications/unsubscribe-auth/AABbefGROPaXzuX0Wnjz-yQ06k2QzZgdks5r7YLQgaJpZM4MFEXK .