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

webpacker:clean is deleting files when they are outputted more than 2 seconds apart #2441

Closed
sbeam opened this issue Jan 23, 2020 · 4 comments · Fixed by #2443
Closed

webpacker:clean is deleting files when they are outputted more than 2 seconds apart #2441

sbeam opened this issue Jan 23, 2020 · 4 comments · Fixed by #2443

Comments

@sbeam
Copy link

sbeam commented Jan 23, 2020

In 4.2.2, the webpacker:clean task is grouping all files that are not mentioned in manifest.json by their last modified time, down to a one-second resolution. This is meant to find very old versions of assets files that can be safely deleted.

After upgrading and deploying to Heroku yesterday, noticed many lines in the build log where assets were being removed just after being compiled (including on brand new app deploys where /public/packs started out empty).

On heroku, and sometimes locally, these groups of files can easily span more than the default of 2. This task now removes all files except from the last 2 seconds.

Here is a example console session from a brand new heroku app where the assets:clean task was skipped, but mimicking the code that would have run if it were left alone:

irb(main):010:0> all_files       = Dir.glob("public/packs/**/*")
irb(main):011:0> manifest_config = Dir.glob("public/packs/manifest.json*")
irb(main):012:0> packs = all_files - manifest_config
irb(main):013:0> pp packs.group_by { |file| File.mtime(file).utc.to_i }.sort.reverse.map(&:first); nil
[1579728071, 1579728056, 1579728055, 1579728054]

webpack outputted files over a period of 17 seconds here, and this clean task would have deleted all the files from the first two groups, including the main .js entrypoints and a lot of other useful stuff.

I think this is due to changes in #2389. Possibly could decrease the resolution of that grouping to something more pessimistic, like 1 minute, or add a min_age param so that it ignores basically new files.

In the meantime have added Rake::Task['assets:clean'].clear to the app to avoid this.

@gauravtiwari
Copy link
Member

Could you please review this? #2443

@sbeam
Copy link
Author

sbeam commented Jan 26, 2020

I left a comment. To be honest I don't think the method of grouping my file mtime is a bulletproof one. Would probably be best to go back to filename matching like sprockets does, which is ugly but is the only reliable way. I can work something up but will take a few days.

@justin808
Copy link
Contributor

@gauravtiwari I just ran into this also! It's quite unexpected that the default uses just 2 different "second times" to group. As far as I know:

  1. Webpack makes no guarantees to output ALL files within the same second.
  2. If you use the multi-compile feature of webpack, you almost will certainly get build files outside of the same second.
  3. If you don't hash some output files, like a bundle for SSR, then you can accidentally delete that bundle.
  4. Some files created by webpack are not in the manifest, depending on the plugin, such as "loadable-components."

For all these reasons, how about this simple fix:

Let's have the versions deletion ONLY apply for files older than AGE.

See comments:
#2443 (review)

I'd be happy to provide a PR for this. It should be totally backward compatible. Yes, some more files can sit around, but is that really all that bad? A little bit of disk space for far less confusion is worth it.

Also, maybe we could rename "keep" to "versions" and provide documentation for that?

Another alternative could be:

  1. Allow an option of -1 for the number of "keep" and make that the default. "-1" for keep would mean not to use the versions.
  2. Maybe even remove the option for keep? We can check for 2 parameters for the age and use that one with a deprecation message.

I completely agree with @sbeam that *we just can't depend on some magical property of webpack outputting files at the same second.

The age defaulting to one hour is much more sensible and least likely to result in countless hours of troubleshooting failed builds.

*My team and the React on Rails Pro clients have spent way too much time troubleshooting failed builds that mysteriously had deleted files.

@flynfish
Copy link

flynfish commented Jul 30, 2020

We just spent most of the day yesterday trying to understand why our deploys kept resulting in 404 on our javascript assets..thankfully found this ticket with the reason why

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants