serve bundles and app under homepage path

[kellyrmilligan]

Verified by adding homepage to package.json, and confirming that it would display the subpath in the terminal, and open the browser with the sub path in the url.

tested by editing both js and css to verify hot loading works the same as before.

Then verified that if I take homepage back out of package.json, it works as intended.

[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.

[kellyrmilligan]

#1582

[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!

[smmoosavi]

with this pr can we pass full url as PUBLIC_URL?

example:
code:

import logo from './logo.svg';
console.log(logo);
console.log(process.env.PUBLIC_URL);

command:

PUBLIC_URL=http://localhost:3000 npm run start

what I expect:

"http://localhost:3000/static/media/logo.5d5d9eef.svg"
"http://localhost:3000"

#1525 (comment)

[Timer]

Can you please re-add the relevant tests from #937?

[kellyrmilligan]

@smmoosavi that command currently doesn't work off of the master branch either, so i'm not expecting it to work with this. with that command, off of master even, it tries to open the browser with http://localhost:3000http://localhost:3000/

but the result when you open the correct url is

http://localhost:3000/static/media/logo.5d5d9eef.svg
App.js:8 http://localhost:3000

under this branch, minus the issue of the command, it prints out

http://localhost:3000/static/media/logo.5d5d9eef.svg
App.js:8 http://localhost:3000

and when you add a homepage with a sub folder:

/subpath/static/media/logo.5d5d9eef.svg
bundle.js:26127 /subpath

[kellyrmilligan]

I synced with upstream master. @smmoosavi @Timer is that command supposed to work then?

[kellyrmilligan]

@Timer what tests are you referring too? it looks like i'm up to date with master, and #937 was merged as part of #1504, so what am I missing? 😕

[smmoosavi]

it tries to open the browser with http://localhost:3000http://localhost:3000/

It's ok for me. In development I always close localhost:3000 (cra page) and open localhost:8000 manually (Django page).

I use cra only for static files (e.g. http://localhost:3000/static/js/bundle.js)

[Timer]

@kellyrmilligan there are tests in that PR that were never merged which check for PUBLIC_URL to be respected on npm start calls

[smmoosavi]

it tries to open the browser with http://localhost:3000http://localhost:3000/

another solution: adding condition only if paths.servedPath starts with / open browser. (else print some text to consle)

[kellyrmilligan]

@Timer I looked through the changes in #937 and it seems like all the tests are there? I found one difference, pushing that up. it was just adding an env variable to one a few of the test runs, but not sure if that is desired or not.

[gaearon]

What should happen if I visit http://localhost:3000/ or http://localhost:3000/anotherpath? I think it would be sensible to automatically redirect to the served path instead in this case.

[gaearon]

Visiting a subpath like http://0.0.0.0:3000/wow/asd/ appears broken (I set http://lol.github.io/wow as the homepage so http://0.0.0.0:3000/wow works):

This breaks any apps relying on client-side routing.

[gaearon]

Files in the public folder still appear to be served from root. This seems like a problem because now the app is not truly self-contained, and this will not match the production behavior.

I would expect that the files in the public folder are served from the subdirectory instead.

[gaearon]

I’d love to get this feature in but we need to fix the bugs:

• Subpaths below the main subpath shouldn't 404
• public folder should serve from main subpath rather than from the root
• Opening the root should redirect to the main subpath

It is important that you test this pull request thoroughly, and carefully check how different existing CRA features are affected by the new behavior.

[gaearon]

It also appears that the end-to-end test is failing. I'm not sure why, but it looks like it never prints The app is running at: for some reason (or maybe the script is doing the wrong thing). I encourage you to try to run it locally and see why it stumbles.

Anyway, I appreciate your work on this so far! I'm afraid I'll have to push this back to 0.11 because I need to cut 0.10 very soon, and unfortunately this feature is not ready.

[sergiocruz]

There's yet another use case I'd like to bring up if possible: the /sockjs-node path.

Do you all think it is possible to namespace that as well?

Right now, it's still trying to find it under root, which unfortunately does not work with my reverse-proxy set up.

Thanks :)

[gaearon]

Good point, I agree any root paths should be namespaced in this case. (Including the new stack frame overlay path.)

[kellyrmilligan]

yes, I need that as well. I thought in my current change it is doing that, but I may be incorrect. i've made some progress today on this and will continue to chip away at it this week.

…

On Mon, May 15, 2017 at 10:48 AM, Dan Abramov ***@***.***> wrote: Good point, I agree any root paths should be namespaced in this case. (Including the new stack frame overlay path.) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1887 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AChQqIl807UHQqjfsmYC0ikna6oPFmA8ks5r6HPWgaJpZM4Mm0dn> .

[kellyrmilligan]

ok, I have the following working locally:

• when you visit the root, it will redirect to the subpath that you defined. I think that's fair, but not sure if you type in some other random sub path if it should redirect.
• serving static files from public/subpath - this also means the user would need to create the folder that matches the sub path. I will update the user guide.
• added the config in webpack dev server to repoint the historyapi fallback to use the servedPath so SPA urls will work now, good catch(totally my bad)

going to run the tests and fix the conflict and push back up.

[gaearon]

I guess I'm okay if we serve 404 on other paths. As long as it's not a 500.

[gaearon]

this also means the user would need to create the folder that matches the sub path

Hmm I don't think so. It should match the way you expect it to work in production. The public is treated as a root but served as a subpath.

[Timer]

It's been so long I'm not sure I fully understand the entire problem here anymore [or my old concerns].
A quick review of the code looks good, but it seems we're serving the files ourselves now (which is fine). Did we disable the dev server from serving them, or just offer another point of entry?

I remember we had concerns on how homepage and PUBLIC_URL function with this change and just in the whole big picture.

At this point in the game, I think it's best to get this to a functioning point, merge it, and then identify pain points and resolve them in a future breaking release.

[kellyrmilligan]

I’m using this right now in day to day dev team of 3, and its working well. Again the main drawback is if you’re using behind a proxy and a sub path, then auto refresh won’t work. I’ll look into the response above to see if there’s more I can do

[Timer]

It seems they're killing off webpack-dev-server -- maybe it'd be best to make our own version with the functionality we need?

[shellscape]

@Timer not a correct assumption there 😄 webpack-dev-server will exist in perpetuity and continue to accept fixes.

[Timer]

Well that's good!

webpack-dev-server has entered a maintenance-only mode. We won't be accepting any new features or major modifications. We'll still welcome pull requests for fixes however, and will continue to address any bugs that arise. Announcement with specifics pending.

Can we get a small expansion on this, or is it a secret? 😛

[shellscape]

@Timer DM me on twitter or invite me to a slack and I'll give you all the dirty deets

[kellyrmilligan]

@shellscape cool to see a clean break and re arch of web pack dev server. From what I can tell with the current wds, we can’t establish a sock js node connection under the root, is that correct?

[shellscape]

@kellyrmilligan its like fight club. what's the first rule 😄

The sockJS implementation is a mess. It takes the best guess as what it should be set at, but there are bugaboos all over the place with that. I'd like to forget about it. It haunts my dreams 👻

[kellyrmilligan]

Ok. So @Timer and @Gaeron I’d like to re sync this Pr with master, and merge if you agree. It only affects folks that want to proxy cra apps under a sub path behind a web server, seems reasonable to me until the new hotness is released and all our problems magically go away #silverbullet

[kellyrmilligan]

Meant @gaearon

[Timer]

Again the main drawback is if you’re using behind a proxy and a sub path, then auto refresh won’t work.

I think I'm fine with this trade-off on first-ship since we're going to coordinate our release very tightly with the new hotness. I don't think we're really worse off than our current position anyway (it just doesn't work now, so we might as well half work).
I'd also like to get the ball rolling that works with the new hotness so it's lined up and ready to go.

[kellyrmilligan]

@Timer synced with master just now.

[shellscape]

@kellyrmilligan @Timer fwiw webpack-serve is now public and kicking. significant minor release on the horizon as well.

[Timer]

@shellscape I'm subscribed to the repo and have been keeping a close eye. I'm sorry for not being more responsive; I'm going through a role change at work right now so my work is sort of doubled down until the transition is complete, thus a lack of time.
I feel strongly about getting this into the 2.0 release -- it has been a pain point for a long time.

[shellscape]

@Timer zero worries my dude! taker care of that IRL and if the opportunity presents, let me know when and if I can help.

[kellyrmilligan]

@Timer do you want to use the new serve module for 2.0 or to get my pr updated for this next release then do it?

[kellyrmilligan]

this is now in #4158

[peterbraden]

What was the status of this? The PR mentioned seems to have died, and this got moved to 'Nope' ?

[bugzpodder]

@peterbraden See #4158.

[peterbraden]

Hi, I was referring to the last comment on that PR: "Thanks for this. I am at a new job now, and with webpack 4 this PR is now old and need to be redone. I think at this point i need to let this PR die yet again"

Not sure how the issues tracker works on this project, but if the issue is closed and the PR dies I wanted to ask if this would fall through the cracks?

Thanks!

[bugzpodder]

I believe at some point create-react-app is going to support this feature but there is no ETA. #4158 is part of the milestone 2.0.0 so rest assured it's not going to fall through the cracks :)

[ram-nadella]

Hi, did this feature make it into 2.0.0? From what I can tell, doesn't look like it did. (I am running react-scripts@2.1.3 and setting the homepage does not appear to have any affect when serving in dev and no mention of this working in the docs either.

@bugzpodder do you have any insight into this?