sgomes
commented
5 years ago Hi @sgomes, thanks for another useful contribution. 😃
Glad to help! 😁
That's a doozy of a use case you've found, and certainly not one I'd thought of. I wonder if this has this happened with other webpack plugins/loaders that cache output, as I took the
find-cache-dirstrategy from things likebabel-loader.
It's quite possible we'll find similar race conditions elsewhere once we apply the changes to this plugin and re-enable parallel builds, yup. It does look like one of those situations where one error could be hiding others down the line. And the fact that it's non-deterministic makes it all harder.
Funnily enough, an option called exactly
cacheDirwas in my first list of ideas when planning this plugin. The build servers I was dealing with at the time ran thenpm installand webpack build processes as separate users in Docker containers, so trying to write a directory tonode_modulesfrom webpack caused a failure. I thought about adding acacheDiroption, but then solved the problem with the "fallback to a temp directory" approach it currently uses. I thought it might be overkill to add an option that possibly only I'd use. Turns out I should have put it in after all! 😄
Hah, well, I'm not surprised you opted against adding it; it does seem like something not a lot of folks would be interested in. That's why I was unsure you'd want to add it to the plugin, too 🙂
Overall I like your solution, but there are some minor things I'd like fixed up before I can merge it.
No problem! I think I've addressed all the comments, but let me know if you'd like any further changes!
gilmoreorless
Hello again, @gilmoreorless ! How have you been? 🙂
We've run into an interesting issue with
moment-timezone-data-webpack-pluginin our setup.We have two separate builds, targeting older and newer browsers, and we used to run them in parallel, on the same server. This was leading to a race condition which caused some builds to break. We tracked it down to the
moment-timezone-data-webpack-plugincache; in some cases, one of the builds would read the directory in an inconsistent state, while the other build was writing to it.We made the builds sequential after that, which solved the issue, but we'd go like to go back to running them in parallel for performance reasons. In order to do that, I propose adding a new
cacheDiroption with this PR.The new option allows a user to specify an absolute path where the cached files will be stored. If not provided, the default auto-generated path will be used instead, as before.
In our case, we'd set it to different values in each build, and thus avoid any collisions. This would solve the problem for us, but I'd love your thoughts on whether it's an appropriate addition to
moment-timezone-data-webpack-plugin.Thank you again for an exceptionally useful plugin! 👍