feat(paths): allow assets to be found using their non digested path

[rainerborene]

After talking to @brenogazzola this morning I decided to implement a more convenient solution. This is a different take to solve the issue #99. Instead of adding a new helper like propshaft_digested_path we are normalizing the manifest artifact and allowing any assets to be found using their non digested path. This works across all Rails helpers like a charm. This means that you can preload and link chunks and assets generated from webpack and other builders.

Say you have a chunk named trumbowyg-ccbb42265c65b6f9a38e.digested.js stored on your app/assets/builds folder now you can reference it using only the filename, for example, preload_link_tag "trumbowyg.js". And the digested path is also stored on the manifest file so that you can serve it through the Rails server or for backwards compatibility. :)

Pretty neat, huh?

[brenogazzola]

Currently Propshaft treats digests it generates differently from digests that the builder generates, when it comes the logical_path.

Generated by propshaft:

logical_path  => application.js
digested_path => application-abcdef123456.js

Generated by a builder:

logical_path  => application-123456abcdef.digested.js
digested_path => application-123456abcdef.digested.js

The hypothesis is that by making Propshaft use the same logical path in both cases, we can solve multiple problems that currently exist when integrating with builders.

@rainerborene I'm a bit crunched for time but as soon as I can I will give this a try in my own production app and then review the PR.

[brenogazzola]

I'm not really a fan of this duble indexing. I think we need to come up with a better solution for this one.

[brenogazzola]

@rainerborene I've been going back and forth with this PR and I'm wondering if we can't considerably simplify everything by separating the predigest from the logical path when initializing the asset.

asset.rb

PREDIGESTED_REGEX = /-([0-9a-zA-Z]{7,128}\.digested)\..+\z/

def initialize(path, logical_path:, version: nil)
  @path         = path
  @digest       = full_path[PREDIGESTED_REGEX, 1]
  @logical_path = digest ? logical_path.sub("-#{digest}", "") : logical_path
  @version      = version
end

server.rb

def extract_path_and_digest(env)
  full_path = Rack::Utils.unescape(env["PATH_INFO"].to_s.sub(/^\//, ""))
  digest    = full_path[PREDIGESTED_REGEX, 1]
  path      = digest ? full_path.sub("-#{digest}", "") : full_path

  [ path, digest ]
end

This way the logical path would never have a digest in it, which means we can eliminate the existing already_digested? which I added a while back and we won't need this PT's undigested path.

What would happen is that a file like application.css would be given a digest as normal, and a file like application-ABCDEF1.digested.js would take on ABCDEF1.digested as the @digest instance variable.

[brenogazzola]

@rainerborene Actually, I'm wondering if we shouldn't move the extract_path_and_digest that the server.rb to somewhere other classes can use.

This way server.rb, asset initializer and resolvers can all use it to get the actual logical path. I think this would also solve the double indexing in the resolver.

[rainerborene]

@brenogazzola Thanks for the feedback! I have implemented the suggested improvements. Let me know if it needs further changes.

[brenogazzola]

Now that we are normalizing the logical path to never have a digest in it, I believe that we can entirely discard the .digested suffix? It's already done its job in telling Propshaft that there is a digest in the file name. This would make non-digested and pre-digested logical paths look the same, and I think it would also incentivize the rest of the code to treat them the same.

By the same token, I think we don't need the @digested_path instance variable. Removing it (and having the logical path always sub instead of using a ternary operator) means extra subs that could have been avoided like you did here, but it also makes the code a bit tidier. At this point where we are still experimenting, I think that would be worth more than the small optimization and make it easier for others since the behavior is always the same.

@logical_path =  logical_path.sub("-#{@digest}.digested", "")

[brenogazzola]

@rainerborene Looking better. Now that I look at it, seems there's a couple of things we can improve further, like completely dropping the .digested suffix and (for now) focusing on simpler, more consistent behavior, instead of extra optimization to avoid .subs.

[brenogazzola]

Hello @rainerborene. Sorry it took me so long to get back. I've done a few extras checks and this looks good. Can you just remove a few extra spaces that were added to keep things clean, and squash again?

I'll merge when you do.

[brenogazzola]

Actually, nevermind. Just noticed you removed the spaces, not added.

Thank for your work on this! 🙇‍♂️

[rainerborene]

🎉

[weilandia]

This commit breaks chunking with esbuild when implemented as suggested here: #48

The reason is likely do the the fact that esbuild creates static chunks that do not have names, i.e. chunk-123, chunk-456, etc.

This is discussed here: evanw/esbuild#1716