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

Server-side files being redirected to using client routing due to registerServiceWorker #3608

Closed
phil-kt opened this issue Dec 16, 2017 · 11 comments

Comments

@phil-kt
Copy link

phil-kt commented Dec 16, 2017

There's this odd bug I'm encountering where using process.env.PUBLIC_URL to link to pdf's in the public folder doesn't work consistently with react-router. It will work fine on localhost, but the moment I deploy it live, the link to the pdf works only once or twice before failing and going to a 404 page.

Also not sure if this bug lies here or with react-router, I couldn't find any reference on their repo. Sorry if it's the wrong spot.

Has anyone else encountered this? For now I'm serving the pdf from within src, but really dislike the complicated path and hash produced by file-loader, so might have to end up ejecting...

I'd also like to note this worked in prior versions of create-react-app as seen here: #775

Is this a bug report?

Yes

Environment

  1. node -v: 8.4.0
  2. npm -v: 5.6.0
  3. react-scripts: 1.0.11
  4. react-router: 4.2.0
  5. react-router-dom: 4.2.2

Steps to Reproduce

  1. Add file to public
  2. Reference it in code using {process.env.PUBLIC_URL + "path to file"}
  3. Create a build and deploy
  4. Click link several times, notice that it stops working

Expected Behavior

Consistently open the file

Actual Behavior

Routed to 404 after using link once.

@phil-kt phil-kt changed the title process.env.PUBLIC_URL and react-router process.env.PUBLIC_URL, pdfs, and react-router Dec 16, 2017
@heyimalex
Copy link
Contributor

Two things. First, you're not providing homepage in package.json. The value of process.env.PUBLIC_URL in production builds is defined using that value, so right now {process.env.PUBLIC_URL + "path/to/pdf"} is evaluating to "" + "path/to/pdf" which is just path/to/pdf.

Second thing is that paths that don't start with a slash are always relative to the current document/route. So when you click on path/to/pdf from home the link works, but when you click on it from /someroute, what actually gets requested is /someroute/path/to/pdf. The way to fix it is to prefix those links with a slash; links with a leading slash are root relative.

Because your whole site is hosted at the root, you can dispense with using PUBLIC_URL all together if your just add the slash to all of your links. You would use PUBLIC_URL if, say, your whole site was hosted under https://philkt.me/mysubsite by setting homepage in pakage.json to https://philkt.me/mysubsite, which would make ${process.env.PUBLIC_URL}/path/to/pdf resolve to https://philkt.me/mysubsite/path/to/pdf.

@phil-kt
Copy link
Author

phil-kt commented Dec 22, 2017

@heyimalex Thanks for the clear explanation!

EDIT: However, it didn't work. I used href='/path/to/pdf/', which in this case is just sitting in the public folder. However, after one click it continues to redirect to 404. So still the same issue.

I think this is the best answer regarding mixing server and client routes: remix-run/react-router#3109

So it seems to be a react-routing issue, so will reclose this as it's not pertinent to react-scripts.

@phil-kt phil-kt closed this as completed Dec 22, 2017
@phil-kt phil-kt reopened this Dec 22, 2017
@phil-kt phil-kt closed this as completed Dec 22, 2017
@phil-kt
Copy link
Author

phil-kt commented Dec 22, 2017

Ok, here's the supposed solution for server routes: delete registerServiceWorker, it makes server routes and react-router not play nicely. However, this didn't work for me with my file.

I'm going to reopen this then, as this part of create-react-app should not alter react-router's default behavior for server-side routes. I'm not quite sure what in registerServiceWorker is the cause.

What I found through testing, is that when using a or an tag, the link would work fine in Firefox Private or Safari Private, only once in other browsers (Firefox, Chrome, Chrome Incognito). Perhaps something in handling the history causes them to fail and render the server route as a client route?

EDIT: It looks like the service worker isn't caching the public files in setOfCachedUrls, could this be the problem?

SECOND EDIT: After re-reading the Getting Started, I realize that server routes aren't supported, but would just like to state it would be nice if they could be, especially in the case of files in /public someone might want to render (in my case a pdf)

I will leave this for a maintainer to close just so they can note the feedback.

Related issue:
remix-run/react-router#5520

@phil-kt phil-kt reopened this Dec 22, 2017
@phil-kt phil-kt changed the title process.env.PUBLIC_URL, pdfs, and react-router Server-side routes being redirected to using client routing due to registerServiceWorker Dec 22, 2017
@phil-kt phil-kt changed the title Server-side routes being redirected to using client routing due to registerServiceWorker Server-side files being redirected to using client routing due to registerServiceWorker Dec 22, 2017
@viankakrisna
Copy link
Contributor

is it possible to prefix the path to your file with _?. This way the service worker wont cache the path. I'm on a mobile right now so sorry for short answer. Basically, service worker generated by CRA will "catch" all paths on second++ visit and serve index.html to all path that is not prefixed with _.

@viankakrisna
Copy link
Contributor

So for example in above case it should be https://philkt.me/_path/to/pdf with a folder _path inside public folder.

@gaearon
Copy link
Contributor

gaearon commented Jan 9, 2018

I suggest to opt out of caching if it's causing you issues.

cc @jeffposnick

@jeffposnick
Copy link
Contributor

You might want to dupe this against #3248 (or #3008).

The change in #3419, if we moved ahead with that, would address it.

@gaearon
Copy link
Contributor

gaearon commented Jan 10, 2018

Fixed by #3419.

@gaearon gaearon closed this as completed Jan 10, 2018
@phil-kt
Copy link
Author

phil-kt commented Jan 11, 2018

@gaearon sweet! Is there any ETA on when the next release will be? Thanks!

@gaearon
Copy link
Contributor

gaearon commented Jan 11, 2018

Within a week I'd say.

@karianpour
Copy link

@viankakrisna I think the '_' behavior is changed to '__' (double underline), am I right?

WeijiaSun pushed a commit to WeijiaSun/personalwebsite that referenced this issue Jul 31, 2018
@lock lock bot locked and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants