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

Asset later in pipeline overwrites previous asset? #740

Open
mrbrdo opened this issue Mar 21, 2022 · 8 comments
Open

Asset later in pipeline overwrites previous asset? #740

mrbrdo opened this issue Mar 21, 2022 · 8 comments

Comments

@mrbrdo
Copy link

mrbrdo commented Mar 21, 2022

I am wondering if this is expected/by design behavior - I have a gem which provides let's say assets/image/dir/img.jpg. Then in my app I provide my own version of this in my app assets. In development this works as expected. However if doing assets precompile, it will use the gem's asset.

I think this behavior is caused by this line, which will keep the last asset that comes with the same logical path:

assets[asset.logical_path] = asset.digest_path

Is this correct behavior or is this a bug?

@brenogazzola
Copy link
Contributor

brenogazzola commented Mar 21, 2022

No it's not the correct behavior. Sprockets has a custom error called DoubleLinkError that is supposed to raise when there's a conflict where two files have the same logical path. Which means Sprockets does not support this (having the same asset path in your app and a gem)

How are you adding the asset file to your pipeline?

@mrbrdo
Copy link
Author

mrbrdo commented Mar 22, 2022

@brenogazzola thanks for a quick reply!
I am using Spree (the Rails commerce platform). Not sure if they patched something here. To be clear the 'stock' file is in a Rails Engine (Spree::Core::Engine). Sprockets version is 4.0.3.
I spent a few hours trying to debug this but so far I wasn't successful.
So simply put, my app and the Spree Engine both have a app/assets/image/dir/img.jpg. They are both in the "files" section of the manifest, and the last one from there is then used in the assets section. Which in this case is the one from the engine since the rails app itself is higher in the list than the gems.
Any idea where I can go further from here?

@brenogazzola
Copy link
Contributor

Changing your file's name or path is, AFAIK, the only way to fix this. Both Sprockets and Propshaft are built in a way where it is impossible to have two files with the same logical path without one getting overwritten (sprockets) or dropped (propshaft).

@mrbrdo
Copy link
Author

mrbrdo commented Mar 22, 2022

Well that's what I want - for it to get overwritten. I don't need both files, I want my file to be used instead of the provided file.
So is it supposed to get overwritten or raise DoubleLinkError?

@brenogazzola
Copy link
Contributor

brenogazzola commented Mar 22, 2022

The correct behavior is the DoubleLinkError. If your app somehow bypassed this raise, whatever ends up happening is unintentional (be it overwritting or dropping) and therefore subject to changes in future versions of Sprockets. It also means that there is nothing in sprockets that let's you tell it which file it should actually keep.

That said, as you noticed, that line uses =. Which means that the file it finds last, is the one that it keeps. So basically, you have to ensure that, in the paths array that tells Sprockets where to look for files to compile, your file comes after the gem's file.

I think (not sure) you can solve that with an initializer.

Rails.application.config.to_prepare do
  asset = Rails.root.join("assets/image/dir/img.jpg")

  Rails.application.config.assets.paths.delete asset
  Rails.application.config.assets.paths << asset
end

This should ensure that your asset is the last in the paths array and therefore the last to go through the =.

@mrbrdo
Copy link
Author

mrbrdo commented Mar 29, 2022

Thanks. I couldn't find any trace of Spree doing any patching of Sprockets, so I'm not sure how this is happening.
For now I have solved it by patching the assignment to be ||=. It's quite dirty but for a proper fix I'd have to find why the DoubleLinkError is not raised and I was not able to do that.

@mrbrdo mrbrdo closed this as completed Mar 29, 2022
@mrbrdo mrbrdo reopened this Nov 21, 2022
@mrbrdo
Copy link
Author

mrbrdo commented Nov 21, 2022

@brenogazzola at the time I took your explanation as proof that it's not Sprockets' fault, but now I'm digging into this again and I'm not sure anymore.

The DoubleLinkError you are mentioning doesn't get triggered because the duplicates are found in different iterations of this loop in Sprockets::Manifest#find:

    # Public: Find all assets matching pattern set in environment.
    #
    # Returns Enumerator of Assets.
    def find(*args, &block)
      # ...
      promises = args.flatten.map do |path|
        # this loop resets `linked_paths` in Sprockets::Base#find_all_linked_assets on each iteration
        Concurrent::Promise.execute(executor: executor) do
          environment.find_all_linked_assets(path).to_a
        end
      end

So actually, there are multiple versions of the same filename that get compiled and are therefore present in @manifest.assets in Sprockets::Manifest. That will just choose the first version, which is the gem's one in my case.

Specifically in my case, these are the args/paths in above Sprockets::Manifest#find where the file is found:

manifest.js # <---- HERE
turbo.js
turbo.min.js
turbo.min.js.map
spree/frontend/all*
spree_backend_manifest.js
spree_emails_manifest.js
spree_frontend_manifest.js # <---- HERE
tinymce-rails.manifest.js
tinymce/*
tinymce.js
doorkeeper/application.css
doorkeeper/admin/application.css
stimulus.js
stimulus.min.js
stimulus.min.js.map
spree_minimax_manifest.js

I think the manifest.js one is the one I have in my app, and the spree_frontend_manifest.js one is from the gem.

In any case, I end up with 2 versions of the same path and then Sprockets will just choose the last one, which in my case is the gem's one, instead of mine.

Do you have any insight on this?

@mrbrdo
Copy link
Author

mrbrdo commented Nov 21, 2022

Also, changing the order of the manifests in Rails.application.config.assets.precompile affects which version will be picked (from the last manifest in that list that includes the path). Note that manifest.js is the first one by default so app's assets have less priority than gem assets.

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

2 participants