-
Notifications
You must be signed in to change notification settings - Fork 105
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
0.10.0 regression in create-react-app integration #74
Comments
@jeffposnick can you try this with v0.11? Also, if i pull that branch used for that pr will i be able to replicate it locally? |
Yes, the regression was introduced in You should be able to reproduce by checking out this commit: https://github.com/jeffposnick/create-react-app/tree/f9b303aa038fedc79adc4ec6e6e94611ef2bae81 Then: $ npm install && npm run build |
@jeffposnick hmm... I have seen this same issue myself progressively more and more lately. I am wondering if this issue is coming from within webpack itself? I'd have to dig to be sure. In terms of timeline of the frequency I've seen this issue and how often webpack/sw-precache-webpack-plugin has done releases... It just points more toward webpack as the cause. I mean, it seems plain as day to me that Try cloning my fork here: https://github.com/hulkish/create-react-app/tree/bugtest I added verbose webpack logging.. the issue now points closer to [16:45:19] Webpack (32%) - Build modules
[16:45:19] Webpack (71%) - Optimize modules (sealing)
[16:45:19] Webpack (72%) - Optimize modules (optimizing)
[16:45:19] Webpack (73%) - Optimize modules (basic module optimization)
[16:45:19] Webpack (74%) - Optimize modules (module optimization)
[16:45:19] Webpack (75%) - Optimize modules (advanced module optimization)
[16:45:19] Webpack (76%) - Optimize modules (basic chunk optimization)
[16:45:19] Webpack (77%) - Optimize modules (chunk optimization)
[16:45:19] Webpack (78%) - Optimize modules (advanced chunk optimization)
[16:45:19] Webpack (32%) - Build modules (~/dev/repos/create-react-app/packages/react-scripts/node_modules/extract-text-webpack-plugin/loader.js??ref--4-0!~/dev/repos/create-react-app/packages/react-scripts/node_modules/style-loader/index.js!~/dev/repos/create-react-app/packages/react-scripts/node_modules/css-loader/index.js??ref--4-2!~/dev/repos/create-react-app/packages/react-scripts/node_modules/postcss-loader/lib/index.js??postcss!~/dev/repos/create-react-app/packages/react-scripts/template/src/index.css)
[16:45:19] Webpack (32%) - Build modules (~/dev/repos/create-react-app/packages/react-scripts/node_modules/extract-text-webpack-plugin/loader.js??ref--4-0!~/dev/repos/create-react-app/packages/react-scripts/node_modules/style-loader/index.js!~/dev/repos/create-react-app/packages/react-scripts/node_modules/css-loader/index.js??ref--4-2!~/dev/repos/create-react-app/packages/react-scripts/node_modules/postcss-loader/lib/index.js??postcss!~/dev/repos/create-react-app/packages/react-scripts/template/src/App.css)
[16:45:19] Webpack (33%) - Build modules (~/dev/repos/create-react-app/packages/react-scripts/node_modules/extract-text-webpack-plugin/loader.js??ref--4-0!~/dev/repos/create-react-app/packages/react-scripts/node_modules/style-loader/index.js!~/dev/repos/create-react-app/packages/react-scripts/node_modules/css-loader/index.js??ref--4-2!~/dev/repos/create-react-app/packages/react-scripts/node_modules/postcss-loader/lib/index.js??postcss!~/dev/repos/create-react-app/packages/react-scripts/template/src/App.css)
[16:45:19] Webpack (33%) - Build modules
[16:45:19] Webpack (79%) - Optimize modules (module and chunk tree optimization)
[16:45:19] Webpack (80%) - Optimize modules (module reviving)
[16:45:19] Webpack (81%) - Optimize modules (module order optimization)
[16:45:19] Webpack (82%) - Optimize modules (module id optimization)
[16:45:19] Webpack (83%) - Optimize modules (chunk reviving)
[16:45:19] Webpack (84%) - Optimize modules (chunk order optimization)
[16:45:19] Webpack (85%) - Optimize modules (chunk id optimization)
[16:45:19] Webpack (86%) - Optimize modules (hashing)
[16:45:19] Webpack (87%) - Optimize modules (module assets processing)
[16:45:19] Webpack (88%) - Optimize modules (chunk assets processing)
[16:45:19] Webpack (89%) - Optimize modules (additional chunk assets processing)
[16:45:19] Webpack (90%) - Optimize modules (recording)
[16:45:19] Webpack (91%) - Optimize modules (additional asset processing)
[16:45:19] Webpack (92%) - Optimize modules (chunk asset optimization)
[16:45:23] Webpack (94%) - Optimize modules (asset optimization)
[16:45:23] Webpack (95%) - Emit files
Total precache size is about 161 kB for 4 resources.
[16:45:24] Webpack: Finished after 8.456 seconds. |
On another note.... I wonder if this could be solved in |
The only thing I can think of is that I started using webpack's built in Tomorrow morning I'll try switching to using Thanks for being so involved @hulkish, if you find anything or have any other ideas keep us posted 😄 |
Re: |
I'm having trouble reproducing the two webpack builds error discovered by @Timer facebook/create-react-app#1728 (comment) I tried with / without dynamic chunks but still can't reproduce it with 0.11.0 |
I think we want this latest version to be in cra btw, it supports webpack chunks. |
I think @hulkish is correct, this doesn't seem like a sw-precache-webpack-plugin issue. I have not been able to reproduce it yet. |
@goldhand actually, I've been able to reproduce it... be sure to check out the particular commit he mentioned: git clone https://github.com/jeffposnick/create-react-app.git
cd create-react-app
git checkout f9b303aa038fedc79adc4ec6e6e94611ef2bae81 Then, you can see the effects of duplicated output by commenting/uncommenting the usage of sw-precache-wepack-plugin (which lives in While I agree that this points to this plugin as the cause, sure... However, I'm not quite convinced just yet. |
Ty @hulkish, I was using the facebook/create-react-app master branch. I don't see that problem there btw. Will test with jeffposnick's repo though |
@goldhand I'm actually working on a pretty big pr for you.. I think that if we are the cause of this issue... then my approach should fix that. My pr will bring:
|
That is awesome @hulkish, Winning! |
How's it coming @hulkish? I'm really excited to see the node vm part of your solution 😄 |
Thanks to @goldhand and especially @hulkish for diving into this! I think it's probably too late in the That being said, the current CC: @prateekbh, who's been developing |
@goldhand @jeffposnick so, the vm solution will work but i kind of hate it, lol. It's really just a way to work around what se-precache doesnt expose independently (fs to support memoryfilesystem. The real challenge I'm finding is the reliability of this approach. It's a fairly fragile approachbased on implementation detail of sw-precache. Which could have really annoying impact on supporting future versions of sw-precache. For example... its not quite so simple as to run it in a VM because there is various other internals of sw-precache that generally make it a very awkward fit to the webpack plugin system. So.. here's what i want to do instead:
Why?
Other notes...
@goldhand im sure you've ran into some of thia before as well. what you think? |
Thanks for the update @hulkish, I will continue to think on this. I've been down the rabbit hole a couple times and haven't been able to find a working solution for webpack-dev-server + sw-precache so I was really interested to see how the vm solution would work. Maybe create branch that uses https://github.com/GoogleChrome/workbox instead of sw-precache (as opposed to rewriting sw-precache). As @jeffposnick mentioned, it will be a lot easier to get your ideas into workbox than sw-precache. I don't think we can re-write sw-precache, and I'm not sure we should remove it from this plugin (considering it's called sw-precache-webpack-plugin) but if we can get webpack-dev-server working with all the sw-precache benefits, I suppose we could try it 😄 I would like to get a fix for create-react-app though and release it in 0.11.1 so create-react-app can have chunk support. Any luck figuring out the issue with the duplicate build? |
@goldhand ur probably right. I actually am not very familiar with workbox at all. But, it does look a bit more plausible per my concerns. However, it still doesn't support a dependency injected file system. They are still using fs directly. As far as the duplicate webpack output goes.. I think this is because we are emitting assets after the compile has already completed via the For now, im thinking ill go back to my solution of using vm just to at least make it work with webpack-dev-server. Ill let you know if that changes. |
@hulkish if you are interested i'll be happy to give u hand with workbox, or if u wanna comeback and do it after you are done with sw-precache |
Workbox's precaching logic assumes that it will be passed an array containing a "manifest" of URLs. The manifest entries can either be an Object, in which case both the [
{url: '/path/to/index.html', revision: '<hash_of_index.html>'},
'/path/to/versioned/file.<hash>.js',
// ...etc.
] It's up to the build tool to generate an appropriate manifest corresponding to the current assets and "inject" it into a well-known placeholder in a source service worker file: const workboxSW = new WorkboxSW();
// The source file contains an empty array as a placeholder:
workboxSW.precache([]);
// After the build process has completed, the final file has [] replaced with the manifest:
// workboxSW.precache([/*actual manifest*/]); The process of generating the manifest is up to the build tool. We shipped Workbox v1.0.0 with some tools that follow the Ideally, the options that the Webpack plugin supported would be roughly equivalent to what other Workbox build tools supported, but if it made sense not to support, e.g., |
@jeffposnick, if I understand correctly, we can generate a manifest (array of filepaths) and provide that to Is As @hulkish mentioned, there is some mismatch with sw-precache and webpack coming from sw-precache's reliance on glob. Webpack knows the all the filepaths (and revisions) so that feature in sw-precache is a hinderance (for usage with webpack). If we can generate the manifest (and avoid the usage of glob) that would really be awesome! |
Basically, yes. If you have a URL that already is versioned (because it has a hash in its URL thanks to Webpack), then you can just pass that string in as an entry in the array. URLs that can't be versioned (like most HTML documents) need to be specified in the
The actual process of spitting out a final |
BEFORE YOU SUBMIT please read the following:
webpack version:
2.x
sw-precache-webpack-plugin version:
0.10.0
Please tell us about your environment:
OSX 10.x
We've been working on a
create-react-app
integration, tracked inhttps://github.com/facebook/create-react-app/pull/1728facebook/create-react-app#1728 (comment) describes a regression when using
v0.10.0
+ of this plugin, in which the Webpack build process is run twice.Looking at the change between
v0.9.1
andv0.10.0
, I guess the issue might have been triggered by something around here: https://github.com/goldhand/sw-precache-webpack-plugin/compare/v0.9.1...v0.10.0?ws=0#diff-1fdf421c05c1140f6d71444ea2b27638R68But I don't know enough about Webpack to say for sure.
The text was updated successfully, but these errors were encountered: