Recompile only modified assets in production

[dbelyaeff]

Working in dev is great. Webpack recompiles and dev-server send to browser only changed files.

But in production it occurs to recompile ALL files. And it WAY TOO LONG.

Is there any method to recompile ONLY CHANGED files in production?

[mdesantis]

You could try using HappyPack: https://github.com/amireh/happypack

[n-rodriguez]

Well it's weird because AFAIK the assets pipeline already handle the case through the manifest.json file
Webpacker creates a manifest.json file too but it always recompiles assets on Capistrano deployments or CI builds :/

[n-rodriguez]

Well actually it relies on a file called .last-compilation-digest-<webpacker env> which is in tmp/cache/webpacker dir (see :

webpacker/lib/webpacker/compiler.rb

Line 83 in 224fc6f

config.cache_path.join(".last-compilation-digest-#{Webpacker.env}")

)
If the file is not found the assets are recompiled.

[n-rodriguez]

If the compilation failed the file is removed :

webpacker/lib/webpacker/compiler.rb

Line 23 in 224fc6f

remove_compilation_digest if !success

.
The thing is (I think) that Capistrano eats compilation errors so the file is removed but you don't know it.
On next deployment the file is not found so compilation happens again.

On CI the issue is the same but "it's normal" : unless you cache the tmp/cache/webpacker dir, the dir is empty and thus the file is not found.

[n-rodriguez]

I had an error in assets compilation but didn't know it until I run rake assets:precompile on my laptop : usabilityhub/rails-erb-loader#63 (comment)

[n-rodriguez]

Hi! I think there is really a bug here.. Because I don't have this behavior in staging environment.
I need to double check this... But I suspect there's something wrong somewhere...

[n-rodriguez]

Yeah : b26e1da

[jpickwell]

@n-rodriguez, you've found the culprit. I think the CI branch of that function should be used all the time. Or, add a new config option (something like, digest_mtime: true), or figure out how to re-purpose the compile option.

Capistrano creates a clone of the latest commit each time you deploy, thus the mtime of the files is always going to be different, and the digest will in turn be different.

[jpickwell]

In case anyone is interested, I created a Capistrano task that sets the atime and mtime of all repo files based on commit author date relative to each file.

https://gist.github.com/jpickwell/c5f1e538f047864283a54032c4825996

[n-rodriguez]

On my side I've monkey patched Webpack ^^ :

# frozen_string_literal: true

# Patch #watched_files_digest method to sort files before digest to
# avoid useless recompilations.
# See: https://github.com/rails/webpacker/issues/1439
module Concerto
  module CoreExt
    module WebpackPatch

      private

        def watched_files_digest
          files = Dir[*default_watched_paths, *watched_paths].reject { |f| File.directory?(f) }
          file_ids = files.map { |f| "#{File.basename(f)}/#{Digest::SHA1.file(f).hexdigest}" }
          Digest::SHA1.hexdigest(file_ids.sort.join('/'))
        end

    end
  end
end

unless Webpacker::Compiler.included_modules.include?(Concerto::CoreExt::WebpackPatch)
  Webpacker::Compiler.send(:prepend, Concerto::CoreExt::WebpackPatch)
end

[ericboehs]

I just noticed this bug yesterday as well. Heroku rebuilds every time and I realized setting CI=true fixed it (as well as changing some paths). I think the solution to always use the file contents rather than mtime is also a good idea. I assume mtime was added for performance but computation of a large app seems instantaneous.

[latortuga]

Yeah : b26e1da

Yeah this is 100% the issue and this has been driving me crazy for months - our deploys tripled in length because everything is recompiled every time. The file contents is what we care about being the same, not when they were modified. Consider this a +1 to use the CI branch version!

[gauravtiwari]

Feel free to make a PR, please :)

[mdesantis]

What about documenting the CI env var loud and clear in the readme? In this way we could keep both the features (mtime check and content check), since mtime check is desirable on standard architectures while content check is desirable on app-files-restoring architectures (such as CI and cloud deploy envs)

[latortuga]

What about documenting the CI env var loud and clear in the readme? In this way we could keep both the features (mtime check and content check), since mtime check is desirable on standard architectures while content check is desirable on app-files-restoring architectures (such as CI and cloud deploy envs)

IMHO I vote that the mtime check not be kept because it is the wrong implementation. If I understand the intent correctly, the digest is designed to check if any file has changed. Checking mtime does not accomplish that. It's merely a proxy.

Say that we checked if the size of the file was the same. Yeah most of the time changes to the file size indicate that the file changed but not all the time. The only 100% accurate way to find out if the file is the same or changed is to check the content of the file.

ETA: This also plays into the "sane defaults" nature of Rails. In order to get reliable asset precompiles that reuse already compiled assets, you have to set an environment variable with an unrelated and obscure name, buried in a submodule's README. Sprockets already does this out of the box so you also might consider this a regression for those "upgrading" to webpack.

[ericboehs]

I vote we also remove the mtime branch of the code.

If we did decide to change it, at the very minimum the variable that is used should be renamed. Since the CI variable isn't name-spaced something like WEBPACKER_CI, it can cause unintended effects to arise in production — or other environments (that should only happen in CI).

[ericboehs]

@n-rodriguez I might have confused you. I originally said "remove the CI branch of the code" but I meant keep the CI branch. I updated my comment.

[jpickwell]

Checking a file's hash is a more sure-fire way of determining modifications, but can be slow. Probably not slow enough to cause problems though.

[ericboehs]

@jpickwell I agree. I wouldn't expect it to be noticeably slower unless there are a massive amount of files.

Perhaps we should get input from @swrobel or @gauravtiwari who originally implemented the mtime branch of the code.

[swrobel]

@ericboehs, I implemented the file hash-based method that's only used if the CI env var is set. The original mtime logic was implemented by @gauravtiwari but it seems like he's open to changing it.

[n-rodriguez]

@swrobel I've been misunderstood ^^ I meant we should compute files hash instead of modification time as I do with my monkey patch : #1439 (comment)

[ericboehs]

@swrobel Thanks for the feedback. Looks like #1743 has been opened to address this issue. Thanks @mdesantis!

[h0jeZvgoxFepBQ2C]

Is the CI variable still usable? We have the nearly the same problem now in webpacker 5.2.1:

#2736

[jonatasdaniel]

Is the CI variable still usable? We have the nearly the same problem now in webpacker 5.2.1:

#2736

I'm with the same problem :/