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

Proposal for capturing digested assets path #99

Closed
rainerborene opened this issue Jul 8, 2022 · 5 comments
Closed

Proposal for capturing digested assets path #99

rainerborene opened this issue Jul 8, 2022 · 5 comments

Comments

@rainerborene
Copy link
Contributor

Hey there! I would like to share some bits of code that we implemented at our company and it could be merged into the official propshaft gem. Right now there is no way to reference digested assets from webpack, rollup.js or any other build system. On the other side, propshaft already does recognize digested assets with .digested. pattern on the filename. The motivation for this feature is to be able to use preload_link_tag or reference any kind of asset generated from external builders. No more talk, here is the code.

# lib/propshaft/digested.rb
module Propshaft
  CACHE = {}

  module Digested
    refine Propshaft::LoadPath do
      def resolve_digested(asset_name)
        Propshaft.resolve_digested_from(asset_name, source: assets_by_path)
      end
    end

    refine Propshaft::Resolver::Static do
      def resolve_digested(asset_name)
        Propshaft.resolve_digested_from(asset_name, source: parsed_manifest)
      end
    end

    refine Propshaft::Resolver::Dynamic do
      def resolve_digested(asset_name)
        load_path.resolve_digested(asset_name)
      end
    end
  end

  def self.resolve_digested_from(asset_name, source:)
    CACHE[asset_name] ||= source.find do |logical_path, _|
      filename, extname = asset_name.split(".")
      pattern = /\A#{filename}-([0-9a-zA-Z]{7,128})\.digested\.#{extname}\z/
      pattern.match?(logical_path)
    end&.first
  end
end

# app/helpers/propshaft_helper.rb
module PropshaftHelper
  using Propshaft::Digested

  def digested_path(path)
    Rails.application.assets.resolver.resolve_digested(path) || raise(Propshaft::MissingAssetError.new(path))
  end
end

Let me know if this sounds good so I can start working on a pull request. :D

@dhh
Copy link
Member

dhh commented Jul 15, 2022

Yes, I like this. cc @brenogazzola

@brenogazzola
Copy link
Collaborator

brenogazzola commented Jul 17, 2022

Sure! @rainerborene Could you also post some sample code of this in action?

From what I'm seeing, the resolve_digested_from will allow us to pass logo.png and have propshaft find logo-DIGEST-digested.png. I'm a bit confused about the digested though. If the builder already generated the file in that format, shouldn't it have already compiled the JS files to replace a reference to logo.png with logo-DIGEST.digested.png? This is what happens when we configure webpack/esbuild to generate entryfiles and chunkfiles with the .digested pattern.

This seems to me something that you’d pass “logo.png”, which is the file the builder wanted, and propshaft would gind “logo-DIGEST.png”, which is what propshaft generated.

@rainerborene
Copy link
Contributor Author

@brenogazzola Not sure if I follow. By digested you mean the digested_path helper method, right? If you want we can have a quick call on Zoom.

@brenogazzola
Copy link
Collaborator

brenogazzola commented Jul 17, 2022

Sorry, let me be more specific (we can try zoom if it’s still confusing)

By digested I meant this line:

pattern = /\A#{filename}-([0-9a-zA-Z]{7,128})\.digested\.#{extname}\z/

I expected it to be:

pattern = /\A#{filename}-([0-9a-zA-Z]{7,128})\.#{extname}\z/

From what I understood of your PR and use case, you have an image file referenced in your JS files as something like logo.png. When you run rails assets:precompile, propshaft will rename the file to logo-DIGEST.png, which means the JS file will not be able to load it. You want to solve that by using a regex to compare file_name with the assets logical paths in the loadpath. Therefore the extra \.digested in the regex seems unnecessary to me. File will only have .digested in their name if the bundler was specifically configured to generate that name. If you are using this new feature in this PR, you are probably not configuring the bunlder to add .digested (I think?)

@rainerborene
Copy link
Contributor Author

rainerborene commented Jul 18, 2022

Sorry, let me be more specific (we can try zoom if it’s still confusing)

Same time zone as yours. Plz ping me at rainerborene at gmail dot com.

If you are using this new feature in this PR, you are probably not configuring the bunlder to add .digested (I think?)

Exact the opposite! Take a look at the output option of the following webpack configuration. As you can see webpack will generate chunks and assets with the .digested. suffix on the filename so that propshaft could understand it and skip the hashing process. The downside is that I cannot link a chunk or asset path using the preload_link_tag (or any other helper) anymore because it weren't digested by propshaft in the first place. That's the issue we're trying to solve here.

module.exports = {
  output: {
    filename: "[name].js",
    sourceMapFilename: "[file].map",
    chunkFilename: "[name]-[contenthash].digested.js",
    assetModuleFilename: "asset-[contenthash].digested[ext]",
    path: path.resolve(__dirname, "app/assets/builds"),
  }
}

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

No branches or pull requests

3 participants