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

Template file and inline render method found for ComponentName #2114

Closed
Paul-Bob opened this issue Oct 1, 2024 · 13 comments
Closed

Template file and inline render method found for ComponentName #2114

Paul-Bob opened this issue Oct 1, 2024 · 13 comments

Comments

@Paul-Bob
Copy link

Paul-Bob commented Oct 1, 2024

Starting with view_component version 3.15.0 and above, some Avo users start to notice some error raised by ViewComponent.

I'm not sure how to reproduce the error, since on my local environment sometimes happens and sometimes not. Same steps on other local environment don't even trigger the error.

I created this issue on Avo and @rickychilcott confirmed that already noticed this error on some parts of the application unrelated to Avo, so I guess it might be a ViewComponent issue.

I wish I could provide more details, but this is all the information I have gathered so far.

Error:

image

@rickychilcott
Copy link
Contributor

The issue is intermittent for me, and I have only noticed it in development. I believe it occurs when the app is reloaded. Last night, while working on a ViewComponent and refreshing my app, it would happen, and then triggering a few reloads would eventually resolve it.

So I think it has to do with code reloading, if I had to guess.

@adrianthedev
Copy link
Contributor

I can't replicate it on my machine.
Without pointing any fingers, I have a hunch it's not particularly Avo, but something in the new VC release.

joelhawksley added a commit that referenced this issue Oct 1, 2024
In service of helping debug #2114,
this change fleshes out the template error messages to include
the current list of templates.
joelhawksley added a commit that referenced this issue Oct 1, 2024
* Add template information to multiple template error messages.

In service of helping debug #2114,
this change fleshes out the template error messages to include
the current list of templates.

* add newline
@joelhawksley
Copy link
Member

Thanks for the report @Paul-Bob! I refactored the ViewComponent compiler in v3.15.0 without changing the public API or test suite, but this kind of failure mode is notoriously difficult to test for.

If you can, can you update to v3.16.0 that includes more debug information with template errors? That should give us more information on what's going on with the hope that we can write a test case that reproduces this bug ❤

@Paul-Bob
Copy link
Author

Paul-Bob commented Oct 1, 2024

Hey @joelhawksley thank you for the prompt help!

Does this ring any bell?

web    | ActionView::Template::Error (Template file and inline render method found for Avo::Cards::CardComponent. There can only be a template file or inline render method per component.
web    | Template file and inline render method found for variant '' in Avo::Cards::CardComponent. There can only be a template file or inline render method per variant.
web    | Templates:
web    | #<ViewComponent::Template:0x000072664b2633f0 @component=Avo::Cards::CardComponent, @type=:file, @this_format=:html, @variant=nil, @lineno=0, @path="/home/bob/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/gems/avo-dashboards-3.13.0/app/components/avo/cards/card_component.html.erb", @extension="erb", @source=nil, @method_name=nil, @defined_on_self=true, @source_originally_nil=true, @call_method_name="call">
web    | #<ViewComponent::Template:0x000072664b262db0 @component=Avo::Cards::CardComponent, @type=:inline_call, @this_format=:html, @variant=nil, @lineno=nil, @path=nil, @extension=nil, @source=nil, @method_name=:call, @defined_on_self=true, @source_originally_nil=true, @call_method_name=:call>):
web    | 
web    | Causes:
web    | ViewComponent::TemplateError (Template file and inline render method found for Avo::Cards::CardComponent. There can only be a template file or inline render method per component.
web    | Template file and inline render method found for variant '' in Avo::Cards::CardComponent. There can only be a template file or inline render method per variant.
web    | Templates:
web    | #<ViewComponent::Template:0x000072664b2633f0 @component=Avo::Cards::CardComponent, @type=:file, @this_format=:html, @variant=nil, @lineno=0, @path="/home/bob/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/gems/avo-dashboards-3.13.0/app/components/avo/cards/card_component.html.erb", @extension="erb", @source=nil, @method_name=nil, @defined_on_self=true, @source_originally_nil=true, @call_method_name="call">
web    | #<ViewComponent::Template:0x000072664b262db0 @component=Avo::Cards::CardComponent, @type=:inline_call, @this_format=:html, @variant=nil, @lineno=nil, @path=nil, @extension=nil, @source=nil, @method_name=:call, @defined_on_self=true, @source_originally_nil=true, @call_method_name=:call>)
web    |     1: <%= render Avo::TurboFrameWrapperComponent.new(params[:turbo_frame]) do %>
web    |     2:   <%= render Avo::Cards::CardComponent.new card: @card %>
web    |     3: <% end %>
web    |   
web    | lib/middleware/account_middleware.rb:32:in `call'

@adrianthedev
Copy link
Contributor

The CardComponent does not have an inline render method, but uses a template.

# frozen_string_literal: true

class Avo::Cards::CardComponent < ViewComponent::Base
  attr_reader :card

  def initialize(card: nil)
    @card = card

    init_card
  end

  def render?
    card.present?
  end

  def init_card
    card.query if card.respond_to? :query
  end
end

@benoittgt
Copy link

benoittgt commented Oct 2, 2024

Hello 👋

With the updated version

Template file and inline render method found for AwxJobOrEventStatusComponent. There can only be a template file or inline render method per component. (ActionView::Template::Error)
Template file and inline render method found for variant '' in AwxJobOrEventStatusComponent. There can only be a template file or inline render method per variant.
Templates:
#<ViewComponent::Template:0x00007f247fece258 @component=AwxJobOrEventStatusComponent, @type=:file, @this_format=:html, @variant=nil, @lineno=0, @path="/home/appuser/app/components/awx_job_or_event_status_component.html.erb", @extension="erb", @source=nil, @method_name=nil, @defined_on_self=true, @source_originally_nil=true, @call_method_name="call">
#<ViewComponent::Template:0x00007f247fecdcb8 @component=AwxJobOrEventStatusComponent, @type=:inline_call, @this_format=:html, @variant=nil, @lineno=nil, @path=nil, @extension=nil, @source=nil, @method_name=:call, @defined_on_self=true, @source_originally_nil=true, @call_method_name=:call>

The component

# frozen_string_literal: true

class AwxJobOrEventStatusComponent < ApplicationComponent # ApplicationComponent has only one helper method

  include Rails.application.routes.url_helpers

  attr_reader :awx_job_or_event

  def initialize(awx_job_or_event:)
    @awx_job_or_event = awx_job_or_event
  end

  def color
    cross = {
      'canceled' => 'gray-600',
      'successful' => 'green-600',
      'pending' => 'gray-600',
      'error' => 'red-600',
      'failed' => 'red-600',
      'running' => 'orange-400',
    }
    cross.fetch(awx_job_or_event.status, cross['canceled'])
  end

end

The caller

<%= render(AwxJobOrEventStatusComponent.new(awx_job_or_event:)) %>

@andrerpbts
Copy link

I'm facing the same issue with some components in my application despite not using Avo on this one. It's intermittent, and after some reloads, it works again. I had to downgrade to 3.14 and lock this version until I get some clarification on how to solve it.

@joelhawksley
Copy link
Member

👋🏻 I've yet to be able to reproduce this issue in dev or test environments. Are any of y'all's apps open source by chance? I'd also be interested in pairing to debug this issue if anyone is up for it ❤

Another option: @Paul-Bob do you have any desire to put together a test app that demonstrates this issue?

@adrianthedev
Copy link
Contributor

Yes @Paul-Bob. I think a test app would be great.

Also, heads up @joelhawksley. This is also an issue that is somehow environment-dependent. Paul set up https://github.com/avo-hq/avo with this scenario, he could see the errors, but I wasn't able to reproduce them.
And he wasn't the only one. We've had customers report this in local and production environments (so it's not just Paul's).

BlakeWilliams added a commit that referenced this issue Oct 5, 2024
This change fixes the bug reported where a component will report that it
has both a template and a `call` method defined despite only having an
HTML template.

I was able to reproduce this bug by pulling in Avo (like reported in the
initial bug report) and running `vegeta` (to trigger parallel requests
since I had a hunch this was a race condition):

```
echo "GET http://localhost:3000/avo/resources/cars/1" | vegeta attack -duration=2s
```

This reliably reproduced the error:

```
ActionView::Template::Error (Template file and inline render method found for Avo::CoverPhotoComponent. There can only be a template file or inline render method per component.
Template file and inline render method found for variant '' in Avo::CoverPhotoComponent. There can only be a template file or inline render method per variant.
Templates:
```

I added _a lot_ of debug logs to understand the race condition that I
thought was occurring, and realized that multiple threads are calling
`gather_templates` _and_ mutating the `@templates` array at the
same.When looking at the old compiler code and realized that this likely
isn't new behavior. This led the investigation towards how we collect
and surface errors or otherwise might modify templates. It turns out
there's a difference in the new and old compiler code after the
refactor:

```ruby
def template_errors
  @__vc_template_errors ||=

def gather_template_errors(raise_errors)
  errors = []
```

_We're not memoizing the errors like we used to_. This is more correct
behavior, but explains how a race condition would make this error case
much more difficult to occur in older versions of the compiler.

This change brings us back to the old behavior by memoizing the errors
we collect in `gather_template_errors` but the `@templates` ivar is
still being mutated. I don't want to change _too much_ in this PR, but a
subsequent change might be wrapping the entire `compile` method in the
`redefinition_lock` (and renaming it to `compile_lock`) to avoid similar
issues in the future.

I did not include a test because it's dfficult to reproduce this race
condition reliably in the test environment.

I believe this _should_ close out #2114
BlakeWilliams added a commit that referenced this issue Oct 5, 2024
This change fixes the bug reported where a component will report that it
has both a template and a `call` method defined despite only having an
HTML template.

I was able to reproduce this bug by pulling in Avo (like reported in the
initial bug report) and running `vegeta` (to trigger parallel requests
since I had a hunch this was a race condition):

```
echo "GET http://localhost:3000/avo/resources/cars/1" | vegeta attack -duration=2s
```

This reliably reproduced the error:

```
ActionView::Template::Error (Template file and inline render method found for Avo::CoverPhotoComponent. There can only be a template file or inline render method per component.
Template file and inline render method found for variant '' in Avo::CoverPhotoComponent. There can only be a template file or inline render method per variant.
Templates:
```

I added _a lot_ of debug logs to understand the race condition that I
thought was occurring, and realized that multiple threads are calling
`gather_templates` _and_ mutating the `@templates` array at the
same.When looking at the old compiler code and realized that this likely
isn't new behavior. This led the investigation towards how we collect
and surface errors or otherwise might modify templates. It turns out
there's a difference in the new and old compiler code after the
refactor:

```ruby
\# old
def template_errors
  @__vc_template_errors ||=

\# new
def gather_template_errors(raise_errors)
  errors = []
```

_We're not memoizing the errors like we used to_. This is more correct
behavior, but explains how a race condition would make this error case
much more difficult to occur in older versions of the compiler.

This change brings us back to the old behavior by memoizing the errors
we collect in `gather_template_errors` but the `@templates` ivar is
still being mutated. I don't want to change _too much_ in this PR, but a
subsequent change might be wrapping the entire `compile` method in the
`redefinition_lock` (and renaming it to `compile_lock`) to avoid similar
issues in the future.

I did not include a test because it's dfficult to reproduce this race
condition reliably in the test environment.

I believe this _should_ close out #2114
@BlakeWilliams
Copy link
Contributor

👋 Hey y'all, I took a peek at the issue and I was able to reliably reproduce this.

It might not be the final version of the PR we end up merging since there's a few other difference in the compiler behavior to talk through and make decisions about, but I believe #2121 should solve the issue y'all are running into here. If you don't mind pulling that branch and validating you no longer run into the issue that would be very helpful.

@adrianthedev
Copy link
Contributor

@BlakeWilliams, you are the man 🙌
Thank you for looking into this.
Paul will check as he is the one that can replicate it.

@Paul-Bob
Copy link
Author

Paul-Bob commented Oct 7, 2024

Hey @BlakeWilliams thank you for looking into this!

If you don't mind pulling that branch and validating you no longer run into the issue that would be very helpful.

I’ve tested using gem "view_component", git: "https://github.com/ViewComponent/view_component.git", branch: "bmw/fix-duplicate-template-error", and the issue seems to be resolved on my end. #2121 seems to have fixed it!

joelhawksley added a commit that referenced this issue Oct 8, 2024
* Fix duplicate template error bug

This change fixes the bug reported where a component will report that it
has both a template and a `call` method defined despite only having an
HTML template.

I was able to reproduce this bug by pulling in Avo (like reported in the
initial bug report) and running `vegeta` (to trigger parallel requests
since I had a hunch this was a race condition):

```
echo "GET http://localhost:3000/avo/resources/cars/1" | vegeta attack -duration=2s
```

This reliably reproduced the error:

```
ActionView::Template::Error (Template file and inline render method found for Avo::CoverPhotoComponent. There can only be a template file or inline render method per component.
Template file and inline render method found for variant '' in Avo::CoverPhotoComponent. There can only be a template file or inline render method per variant.
Templates:
```

I added _a lot_ of debug logs to understand the race condition that I
thought was occurring, and realized that multiple threads are calling
`gather_templates` _and_ mutating the `@templates` array at the
same.When looking at the old compiler code and realized that this likely
isn't new behavior. This led the investigation towards how we collect
and surface errors or otherwise might modify templates. It turns out
there's a difference in the new and old compiler code after the
refactor:

```ruby
\# old
def template_errors
  @__vc_template_errors ||=

\# new
def gather_template_errors(raise_errors)
  errors = []
```

_We're not memoizing the errors like we used to_. This is more correct
behavior, but explains how a race condition would make this error case
much more difficult to occur in older versions of the compiler.

This change brings us back to the old behavior by memoizing the errors
we collect in `gather_template_errors` but the `@templates` ivar is
still being mutated. I don't want to change _too much_ in this PR, but a
subsequent change might be wrapping the entire `compile` method in the
`redefinition_lock` (and renaming it to `compile_lock`) to avoid similar
issues in the future.

I did not include a test because it's dfficult to reproduce this race
condition reliably in the test environment.

I believe this _should_ close out #2114

* Fix tests, raise errors for compiled components with errors

* There isn't always a default template when there are errors.

* Make standard happy

ugh

* Remove debug code

* Fail compilation when errors present

* Revert default template change

* Add informative comment

* Add changelog

---------

Co-authored-by: Joel Hawksley <[email protected]>
@joelhawksley
Copy link
Member

Closed by #2121

boardfish pushed a commit to reeganviljoen/view_component that referenced this issue Oct 11, 2024
* Fix duplicate template error bug

This change fixes the bug reported where a component will report that it
has both a template and a `call` method defined despite only having an
HTML template.

I was able to reproduce this bug by pulling in Avo (like reported in the
initial bug report) and running `vegeta` (to trigger parallel requests
since I had a hunch this was a race condition):

```
echo "GET http://localhost:3000/avo/resources/cars/1" | vegeta attack -duration=2s
```

This reliably reproduced the error:

```
ActionView::Template::Error (Template file and inline render method found for Avo::CoverPhotoComponent. There can only be a template file or inline render method per component.
Template file and inline render method found for variant '' in Avo::CoverPhotoComponent. There can only be a template file or inline render method per variant.
Templates:
```

I added _a lot_ of debug logs to understand the race condition that I
thought was occurring, and realized that multiple threads are calling
`gather_templates` _and_ mutating the `@templates` array at the
same.When looking at the old compiler code and realized that this likely
isn't new behavior. This led the investigation towards how we collect
and surface errors or otherwise might modify templates. It turns out
there's a difference in the new and old compiler code after the
refactor:

```ruby
\# old
def template_errors
  @__vc_template_errors ||=

\# new
def gather_template_errors(raise_errors)
  errors = []
```

_We're not memoizing the errors like we used to_. This is more correct
behavior, but explains how a race condition would make this error case
much more difficult to occur in older versions of the compiler.

This change brings us back to the old behavior by memoizing the errors
we collect in `gather_template_errors` but the `@templates` ivar is
still being mutated. I don't want to change _too much_ in this PR, but a
subsequent change might be wrapping the entire `compile` method in the
`redefinition_lock` (and renaming it to `compile_lock`) to avoid similar
issues in the future.

I did not include a test because it's dfficult to reproduce this race
condition reliably in the test environment.

I believe this _should_ close out ViewComponent#2114

* Fix tests, raise errors for compiled components with errors

* There isn't always a default template when there are errors.

* Make standard happy

ugh

* Remove debug code

* Fail compilation when errors present

* Revert default template change

* Add informative comment

* Add changelog

---------

Co-authored-by: Joel Hawksley <[email protected]>
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

7 participants