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

Using asset_path in component previews #1093

Open
Spone opened this issue Oct 11, 2021 · 15 comments
Open

Using asset_path in component previews #1093

Spone opened this issue Oct 11, 2021 · 15 comments

Comments

@Spone
Copy link
Collaborator

Spone commented Oct 11, 2021

Steps to reproduce

  • put image.png in app/assets/images/image.png
  • create an example component that accepts an image argument, with a preview file
  • in the preview file, add render ExampleComponent.new(image: image_path("image.png")
  • this will trigger a NoMethodError
  • add include ActionView::Helpers::AssetUrlHelper at the top of the preview class
class ExampleComponent < ApplicationComponent
  def initialize(image:)
    @image = image
  end

  def call
    image_tag @image
  end
end
class ExampleComponentPreview < ViewComponent::Preview
  include ActionView::Helpers::AssetUrlHelper

  def default
    render(ExampleComponent.new(image: image_path("image.png")))
  end
end

Expected behavior

The preview renders the image with path /assets/image-80b04a6277839296f66f0e7de1fb03a3061322fad2273b18a375bf2f5381c41f.png.

Actual behavior

The preview renders but the image path is /images/image.png.

System configuration

Rails version: 6.1.4.1

Ruby version: 2.7.4

Gem version: 2.40.0

@joelhawksley
Copy link
Member

@Spone I forgot about this issue. Does #1119 close it?

@Spone
Copy link
Collaborator Author

Spone commented Nov 5, 2021

@joelhawksley nope, I still have the issue on main... but I no longer have to include ActionView::Helpers::AssetUrlHelper for the preview to render.

The image path is still wrong though:

<img src="/images/image.png">

instead of:

<img src="/assets/image-80b04a6277839296f66f0e7de1fb03a3061322fad2273b18a375bf2f5381c41f.png">

@ellnix
Copy link

ellnix commented Apr 13, 2022

TL;DR
ActionView::Helpers::AssetUrlHelper does not provide the correct logic for loading an asset through the asset pipeline (Sprockets). The easiest (incredibly ugly) workaround is to include the sprockets-rails helper and its configuration into your preview class.

class ExampleComponentPreview < ViewComponent::Preview
    include Sprockets::Rails::Helper
    self.debug_assets = Rails.application.config.assets.debug
    self.digest_assets = Rails.application.config.assets.digest
    self.assets_prefix = Rails.application.config.assets.prefix
    self.assets_precompile = Rails.application.config.assets.precompile
    self.assets_environment = Rails.application.assets
    self.assets_manifest = Rails.application.assets_manifest
    self.resolve_assets_with = Rails.application.config.assets.resolve_with
    self.check_precompiled_asset = Rails.application.config.assets.check_precompiled_asset
    self.unknown_asset_fallback = Rails.application.config.assets.unknown_asset_fallback
    # Expose the Rails.application precompiled asset check to the view
    self.precompiled_asset_checker = -> logical_path { Rails.application.asset_precompiled? logical_path }
  
   ...
end

This will set up the context in your Preview class the same way sprockets-rails currently sets up the context in ActionView::Base which allows you to use asset helpers in views. Not only is this a lot of ugly boilerplate code (which you should at least wrap in a module) but it is liable to break if sprockets-rails changes its railtie. For more information and better ideas keep reading.


Disclaimer: I do not have the deepest understanding of the inner workings of Rails and even less of an understanding of Sprockets. These are observations I made from reading the source code, so expect parts of my interpretations to be incorrect.


image_path along with other asset helpers, unless passed a truthy value in the skip_pipeline: option will get the path of the asset from a method in the same module named compute_asset_path. If told to skip the pipeline the public_compute_asset_path method is used instead, which appears to be intended to be used for assets manually placed in the public/ folder.

By default these path resolving methods are actually aliases, which is why image_path acts as though it was passed skip_pipeline: true and resolves to /images/image.png. compute_asset_path is meant to be overridden by the gem that manages the asset pipeline (in your case Sprockets).

sprockets-rails has its own compute_asset_path method in a helper module that gets included (and configured) in ActionView::Base on load by a the sprockets-rails railtie. The new compute_asset_path method relies on configuration options included in the ActionView context by said railtie. The above solution works by basically copying the loading logic in the railtie.

In my opinion, the dynamic nature of the ActionView::Base context ought to disqualify solutions like the one above (and the current include ActionView::Helpers::AssetTagHelper in ViewComponent::Preview, which can lead its users to believe that they can use asset helpers in a preview). We should either disallow view helpers in previews or commit to allowing helper methods to be used via a helpers accessor initialized with ViewComponentsController's view_context, similar to the way you might delegate :method, to: :helpers in a component. This way we can stay consistent and avoid misleading users by only including a subset of ActionView::Base.

@unikitty37
Copy link

Please don't disallow view helpers in previews — I need to use them, and I suspect I'm not the only one.

See #1451 for more detail on what I'm doing (though allowing component tests to do expect(page).to have_rendered_component(ExampleComponent) would also solve my problem here, but that's a different issue :)

@Spone
Copy link
Collaborator Author

Spone commented Apr 7, 2023

I got bitten by this again today. @joelhawksley how do you want to tackle this?

@joelhawksley
Copy link
Member

@Spone I think the first step we should take is to see if these same helpers work in ActionMailer previews, and if so, how they work there. Would you be interested in taking a look?

@Spone
Copy link
Collaborator Author

Spone commented Apr 24, 2023

I guess the relevant part of ActionMailer is here: https://github.com/rails/rails/blob/main/actionmailer/lib/action_mailer/railtie.rb#L38-L40

@joelhawksley
Copy link
Member

@Spone would you like to take a crack at seeing if that works for us?

@gap777
Copy link

gap777 commented Sep 13, 2023

Just spent a few hours banging my head against the wall because we used image_tag inside our view component, and tried to use a component (rendering-only) test built using render_inline. In our CI env, when component tests are executed, we don't have any assets built, so when image_tag is encountered, manifest.js is sought (in order to determine the image src path), and never found (because in this non-system test, we didn't have assets precompiled).

🤦

@reeganviljoen
Copy link
Collaborator

reeganviljoen commented Sep 16, 2023

@gap777 @Spone can we close this issue then ?

@Spone
Copy link
Collaborator Author

Spone commented Sep 16, 2023

Why? Is it fixed? 🧐

@gap777
Copy link

gap777 commented Sep 16, 2023

@reeganviljoen @Spone is right -- this isn't really fixed.

I got to this issue because what I was doing was related to

  1. view components
  2. render inline
  3. My belief that I shouldn't need assets from asset pipeline, since a browser wasn't being launched

Perhaps my comment should drive a separate ticket / PR that updates the documentation around testing view components... even if you're using render_inline, IF you're using any view helpers that rely on the assets pipeline (like image_tag), you're going to need assets available for that test to execute/pass.

@reeganviljoen
Copy link
Collaborator

@gap777 personally when ever I run tests in ci I precomile the assets as a setup step, maybe we should state that in docs

@gap777
Copy link

gap777 commented Sep 16, 2023

@reeganviljoen We have 2 parallel workflows in our CI: those requiring the assets, and those that do not (unit tests, static code analysis/rubocop, etc). My mistake was that I had tried to reduce the size of the one and move these render_inline based tests into the other.
Yes, I think documentation calling attention to this idea would be helpful... all view component tests should be executed in an environment that has assets available, since, after all, these are view tests!

@reeganviljoen
Copy link
Collaborator

reeganviljoen commented Sep 16, 2023

@gap777, we could also add a check in rener_inline to compile the assets if not present or throw an exception. @Spone, what do you think ?

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

No branches or pull requests

6 participants