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

service worker swallows requests for files added to public/ #3008

Closed
chee opened this issue Aug 26, 2017 · 18 comments
Closed

service worker swallows requests for files added to public/ #3008

chee opened this issue Aug 26, 2017 · 18 comments

Comments

@chee
Copy link

chee commented Aug 26, 2017

Is this a bug report?

i can't tell, i suppose not.

Which terms did you search for in User Guide?

"service worker"

explanation

on my blog, the posts are stored in a file called posts.json, which i keep in public/.
the route for printing out a handsome html list of posts data is /posts.

the posts themselves are built from markdown files that live in, for example,
public/posts/an-example-slug.md. the url for the react rendered version of that
post is /posts/an-example-slug.

a cute feature of that has been that if you would like to view the source of one of
the posts, you can just add .md to the end of the url and there it
appears in all its monospace fancy.

i was working on some feature changes last night and this morning, and i've
upgraded react-scripts and have my setup working nicely with the service worker.
except now the service worker jumps in between me and my public/ file request.

i'm just now realising it is also eating requests for my feed.rss and manifest.json and i'm beginning to wonder if i have done something terribly wrong.

can you think of anything i can do without ejecting?
i suppose i can generate my own service worker based on the lodash template.

related

#2253
#2714

@chee
Copy link
Author

chee commented Aug 26, 2017

now that i'm reading the generated service-worker.js it would seem it doesn't even try to respond on fetches for files it's never heard of

@Timer
Copy link
Contributor

Timer commented Aug 26, 2017

/cc @jeffposnick

@chee
Copy link
Author

chee commented Aug 26, 2017

i've just checked, and if i sneak if (e.request.url.endsWith(".json")) { return } into service-worker.js at the beginning of the fetch eventListener block, it serves me the file.

i would have expected any static file that is not explicitly cached to be ignored by the service worker, i've traditionally been using build as my Express.static.

though i am welcome to the idea that this is all my fault.

once i have a temporary workaround for my immediate problem i'll try to recreate the issue with a fresh cra.

@chee
Copy link
Author

chee commented Aug 26, 2017

yeah so it happens there too.

steps to reproduce

create-react-app service-work-me
cd service-work-me
echo '<!doctype html><title>oh no</title>' > public/404.html
echo '["some", "json"]' > public/mushroom.json
npm  run build
npm i -g serve
serve -s build

then:

(it sends down the contents of index.html, then the js kicks in)

but in a browser that does not support service workers, http://localhost:5000/404.html and http://localhost:5000/mushroom.json display their own content.

perhaps this is expected or necessary behaviour because you don't know what is and isn't a route in the app.

though serve and Express will try to serve the file in the directory first, won't they?.

as it seems this would require configuration of the navigateFallbackWhitelist, the recommendation will probably be to eject.

though there is a precedent for convention here, the service worker ignores firebase urls.

i will make mine ignore routes that have file extensions, as i will still want to be able to link people to files hosted there even if they've been on the site before.

@gaearon
Copy link
Contributor

gaearon commented Aug 26, 2017

Thanks for repro steps! Leaving this to @jeffposnick to comment on.

@jeffposnick
Copy link
Contributor

jeffposnick commented Aug 27, 2017

I believe that this is fundamentally the same topic as #2894 (comment)

It's reasonable to expect that anything you navigate to that exists as a static file in public/ will Just Work, and after looking at the docs for https://github.com/goldhand/sw-precache-webpack-plugin#configuration, I think I see how we can get that set up, despite the fact that public/* doesn't get picked up in the Webpack build.

@goldhand, it would be great if you could confirm, but I believe modifying the SWPrecacheWebpackPlugin config to add in

{
  staticFileGlobs: ['./public/**/*'],
  mergeStaticsConfig: true,
  stripPrefix: 'public/',
}

alongside the other options should do the trick.

I can't test that out at the moment, but I could in a day or so, unless anyone else does first.

@chee
Copy link
Author

chee commented Aug 28, 2017

you're quite right, @jeffposnick, this is the same topic.

adding that config doesn't quite do the trick. those files are now included in the precache, with the perhaps surprising effect that they will all be downloaded and cached during the first visit to the site, but they are still not available by a direct navigate.

though this seems to be because the public prefix was not stripped off

@chee
Copy link
Author

chee commented Aug 28, 2017

@jeffposnick's config does do the trick, with stripPrefix changed from public/ to ./public

{
  staticFileGlobs: ['./public/**/*'],
  mergeStaticsConfig: true,
  stripPrefix: './public',
}

@jeffposnick
Copy link
Contributor

Great, glad to hear that works.

I think that would be safe to add to the default config, but it would be awesome to get a confirmation when @goldhand is around, because these are options that are specific to his Webpack plugin.

I'm also not in a position to file a PR for a bit while traveling, so perhaps someone else will want to step in there...

@chee
Copy link
Author

chee commented Aug 28, 2017

thanks for working on this while you're traveling, @jeffposnick !

i've filed a PR with your changes, with a note that it's waiting for confirmation from @goldhand.

someone may also like to fix the language in the comment.

@goldhand
Copy link
Contributor

Sorry for the delay @chee, your changes are 👍

@gaearon
Copy link
Contributor

gaearon commented Sep 1, 2017

Instead of precaching everything in public, can instead force requests to it always go to network? Somehow.

@chee
Copy link
Author

chee commented Sep 1, 2017

@goldhand no problem ! thank you

@gaearon there's some discussion of that here: #3024 (comment)

in summary: that's what i would prefer, it would be much simpler if sw-precache took a navigateFallbackBlacklist as well as a navigateFallbackWhitelist.

i think the surprise in having every file you've put in public/ be downloaded and cached for the rest of time on the first access to the site is probably equal to the surprise of not being able to navigate to them at all.

@jeffposnick
Copy link
Contributor

Thanks for taking the lead on this, @chee. I'll chime in on the open PR regarding alternatives to precaching the entirety of public/ by default.

@aaronshaf
Copy link
Contributor

I am having problems accessing /sitemap.xml. It was built to the public directory, and resulted in being in the build directory, but accessing /sitemap.xml in my browser results in showing index.html via the service worker.

@jeffposnick
Copy link
Contributor

@aaronshaf, what you describe is related to this PR and to #3248.

Once #3248 is fixed and deployed, you should no longer see index.html in response to navigation requests for other URLs.

@gaearon
Copy link
Contributor

gaearon commented Jan 10, 2018

Fixed by #3419.

@vivekiyer114
Copy link

vivekiyer114 commented Nov 22, 2018

Here's a quick fix for this.

// navigateFallback: publicUrl + '/index.html'

I just commented the navigateFallback function.
Please let me know if there any demerits for this.

@lock lock bot locked and limited conversation to collaborators Jan 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants