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

Generated service worker uses file paths under Windows 10 #2235

Closed
jorispz opened this issue May 19, 2017 · 20 comments · Fixed by #2255
Closed

Generated service worker uses file paths under Windows 10 #2235

jorispz opened this issue May 19, 2017 · 20 comments · Fixed by #2255

Comments

@jorispz
Copy link

jorispz commented May 19, 2017

Can you reproduce the problem with latest npm?

Yes, I upgraded to the npm version 4.6.1 and the proble is reproducible

Description

Under windows, in a brand new project created with create-react-app, the service worker uses the location on disk of the files to be cached in its cache configuration.

Expected behavior

The service worker uses the proper URLs for caching configuration

Environment

[email protected]
node v6.10.1
npm 4.6.1

Windows 10
Any browser (not a browser issue)

Reproducible Demo

Create a brand new app with create-react-app in the environment above, run npm run build, and check the contents of service-worker.js. It contains file paths, such as

var precacheConfig = [
  ["C:/dev/temp/pwatest/build/index.html", "df24d8c1abce89d06bc00d2023cdad0c"],
  [
    "C:/dev/temp/pwatest/build/static/css/main.9a0fe4f1.css",
    "3473922d6aed4c20bb69846d6027cacf"
  ],
  [
    "C:/dev/temp/pwatest/build/static/js/main.23e5d2a8.js",
    "d0f59b7dffbc3ff4835b038c8f3a4fdc"
  ],
  [
    "C:/dev/temp/pwatest/build/static/media/logo.5d5d9eef.svg",
    "5d5d9eefa31e5e13a6610d9fa7a283bb"
  ]
]
@gaearon
Copy link
Contributor

gaearon commented May 19, 2017

Paging @jeffposnick for help.

@jeffposnick
Copy link
Contributor

That's not good.

I just found goldhand/sw-precache-webpack-plugin#11 which indicates that this is a known problem with the Webpack wrapper to sw-precache. CC: @goldhand, the maintainer of that plugin about that.

I'm going to experiment and see if it's possible to pass in a safe, default stripPrefix configuration option that would accommodate the build setup without having to wait on a change to the underlying Webpack plugin.

@gaearon
Copy link
Contributor

gaearon commented May 19, 2017

Given that many other tools are broken on Windows, we have historically made it a point to ensure that every Create React App feature is supported and works well on Windows. This issue is a high priority for us and any help or temporary workaround would be welcome (even a hacky one).

@gaearon gaearon added this to the 1.0.1 milestone May 19, 2017
@jeffposnick
Copy link
Contributor

Definitely understood. I'm actively investigating whether there's a clean workaround short of requiring a new version of sw-precache-webpack-plugin.

@goldhand
Copy link
Contributor

@jeffposnick I've heard that setting stripPrefix: path.join(__dirname, 'dist').replace(/\\/g,"/") works for windows uses (as a temporary windows solution). I am looking into a fix that will work on windows without configuration.

@gaearon
Copy link
Contributor

gaearon commented May 19, 2017

I have a Windows laptop so I can verify any fix if you explain how to check that it works.

@goldhand
Copy link
Contributor

@gaearon In the generated service worker, the var precacheConfig should strip out the absolute paths from the webpack bundles and replace it with the webpack.publicPath setting. In the demo the absolute windows paths are still there and were never replaced.

@gaearon
Copy link
Contributor

gaearon commented May 19, 2017

Do I understand correctly that I'd need to adjust that to be stripPrefix: path.join(__dirname, 'build').replace(/\\/g,"/")? Given that build is where we put the output.

@gaearon
Copy link
Contributor

gaearon commented May 19, 2017

Or, rather, just path of my build folder (hierarchy is a bit different in this project).

@goldhand
Copy link
Contributor

sorry, yes just the path to the build folder. I think path.join(__dirname, 'build') will point to the build folder but you're right, you just want to strip the path to the build folder (where ever that is).

@gaearon
Copy link
Contributor

gaearon commented May 19, 2017

OK, I think #2255 helps in my testing. Thanks everybody for raising this and suggesting fixes.
Would be nice to have a more permanent fix for this upstream.

@goldhand
Copy link
Contributor

goldhand commented May 19, 2017

@gaearon, I will be working on a permanent fix.

@jeffposnick
Copy link
Contributor

Thanks so much for stepping in with that workaround, @goldhand!

As a heads-up, I've been trying to confirm that it works as intended for common c-r-a use cases, and I am seeing an issue where using that stripPrefix setting that was just merged will prevent the path of the custom homepage setting from package.json from being appended to the output URLs.

I am going to continue trying different scenarios, but I wanted to point out that this setting appears to be an issue for homepage users.

@jeffposnick
Copy link
Contributor

{
  stripPrefix: (paths.appBuild + '/').replace(/\\/g, '/'),
}

is roughly equivalent, but with the additional property of stripping out the leading /, making all the URLs relative to the location of service-worker.js. Since the service worker will always be deployed at the correct sub-directory based on the homepage config, using relative paths should be a safe workaround.

@gaearon
Copy link
Contributor

gaearon commented May 19, 2017

Ah, thanks for catching that.

@gaearon
Copy link
Contributor

gaearon commented May 19, 2017

I assume just paths.appBuild.replace(/\\/g, '/') + '/' would work, right?

@jeffposnick
Copy link
Contributor

jeffposnick commented May 19, 2017 via email

@gaearon
Copy link
Contributor

gaearon commented May 19, 2017

[email protected] should be out with a fix.
Can you please verify it helps?

@jorispz
Copy link
Author

jorispz commented May 19, 2017

Tested and I can confirm it works for me.

Thanks to all involved, this is an impressive turnaround time!

@goldhand
Copy link
Contributor

Just published 0.9.2 on npm with the windows fix. It's here: goldhand/sw-precache-webpack-plugin#77
Will release that fix along with a fix for goldhand/sw-precache-webpack-plugin#74 in 0.11.1 if you would rather just wait for that to come out.

@lock lock bot locked and limited conversation to collaborators Jan 21, 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.

4 participants