ViewComponent / view_component

A framework for building reusable, testable & encapsulated view components in Ruby on Rails.
https://viewcomponent.org
MIT License
3.23k stars 416 forks source link

Using asset_path in component previews #1093

Open Spone opened 2 years ago

Spone commented 2 years ago

Steps to reproduce

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 commented 2 years ago

@Spone I forgot about this issue. Does https://github.com/github/view_component/pull/1119 close it?

Spone commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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

See https://github.com/github/view_component/discussions/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 commented 1 year ago

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

joelhawksley commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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

gap777 commented 12 months ago

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 commented 11 months ago

@gap777 @Spone can we close this issue then ?

Spone commented 11 months ago

Why? Is it fixed? 🧐

gap777 commented 11 months ago

@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 commented 11 months ago

@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 commented 11 months ago

@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 commented 11 months ago

@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 ?