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

Components with only variant template crashes the app #1956

Open
sigra opened this issue Jan 8, 2024 · 7 comments
Open

Components with only variant template crashes the app #1956

sigra opened this issue Jan 8, 2024 · 7 comments

Comments

@sigra
Copy link

sigra commented Jan 8, 2024

I've started to upgrade from 3.2.0 to 3.9.0 and my application stopped launching. Downgrading version by version I found that the issue starts from 3.5.0.

The app crashes with the error like:

NameError:
  undefined method `call' for class `ExampleComponent'

My component doesn't have template without variant and I sure it's an issue. Seems that Compiler#define_render_template_for expects that component class always has :call method defined:

component_class.define_method("_call_#{safe_class_name}", component_class.instance_method(:call))

But this method isn't defined because my component has only "variant" template:

templates.each do |template|
method_name = call_method_name(template[:variant])
redefinition_lock.synchronize do
component_class.silence_redefinition_of_method(method_name)
# rubocop:disable Style/EvalWithLocation
component_class.class_eval <<-RUBY, template[:path], 0
def #{method_name}
#{compiled_template(template[:path])}
end
RUBY
# rubocop:enable Style/EvalWithLocation
end
end

Steps to reproduce

  1. Make a component using subdirectory template structure
app/components
├── ...
├── example_component.rb
├── example_component
|   ├── example_component.html+table.haml
├── ...
  1. Run the app

System configuration

Rails version:
6.1.7.5

Ruby version:
3.2.2

Gem version:
3.5.0

@reeganviljoen
Copy link
Collaborator

reeganviljoen commented Jan 8, 2024

@sigra the reason you get this error is because you aren't using a variant in your request so the component will not be rendered with a variant, and in that case it needs a default template without a variant, if you dont want the component to render without a variant, try doing a check on the variant in the render? method

@boardfish
Copy link
Collaborator

Thanks for the diagnosis, @reeganviljoen! Knowing this, we could start returning a helpful error here with similar advice to what you've provided just there.

@reeganviljoen
Copy link
Collaborator

@boardfish I can get that working when I get some time

@sigra
Copy link
Author

sigra commented Jan 9, 2024

Thanks for reply, @reeganviljoen . Patching the render? method didn't help. Because the error occurs earlier in the inherited block where compile method is called.

class ExampleComponent < ApplicationComponent
  def render?
    raise 'Unreachable code'
  end
end

Rails console output:

irb(main):001:0> ExampleComponent
/Users/sigra/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/view_component-3.9.0/lib/view_component/compiler.rb:110:in `instance_method': undefined method `call' for class `ExampleComponent' (NameError)

      component_class.define_method(:"_call_#{safe_class_name}", component_class.instance_method(:call))
                                                                                ^^^^^^^^^^^^^^^^
Did you mean?  caller

Backtrace:

.../view_component-3.9.0/lib/view_component/compiler.rb:111:in `instance_method'
.../view_component-3.9.0/lib/view_component/compiler.rb:111:in `define_render_template_for'
.../view_component-3.9.0/lib/view_component/compiler.rb:86:in `compile'
.../view_component-3.9.0/lib/view_component/base.rb:562:in `compile'
.../view_component-3.9.0/lib/view_component/base.rb:497:in `inherited'

I know that I can create empty example_component.haml.html template and it will fix the problem. But as for me it's a little bit hacky way, because my app doesn't need to have this template. So, I assume it's a bug.

As for me the most obvious way is to check is method call defined or not by using the call_defined? method.

def call_defined?
component_class.instance_methods(false).include?(:call) ||
component_class.private_instance_methods(false).include?(:call)
end

Suggested fix:

      if call_defined?
        component_class.define_method(:"_call_#{safe_class_name}", component_class.instance_method(:call))
      end

@reeganviljoen
Copy link
Collaborator

@sigra we really appreciate your debug samples and reproduction, this will certainly help, would you be able to write a failing test for this situation so someone can use that as a guideline to fix this bug ?

sigra added a commit to sigra/view_component that referenced this issue Jan 10, 2024
@sigra
Copy link
Author

sigra commented Jan 10, 2024

@reeganviljoen will be it enough? sigra@0b80539

➜  view_component git:(main) ✗ bundle exec appraisal rake

>> BUNDLE_GEMFILE=/Users/sigra/projects/view_component/gemfiles/rails_6.1.gemfile bundle exec rake
/Users/sigra/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/activesupport-6.1.7.6/lib/active_support/core_ext/class/subclasses.rb:30: warning: method redefined; discarding old subclasses
/Users/sigra/projects/view_component/lib/view_component/compiler.rb:110:in `instance_method': undefined method `call' for class `VariantComponentWithoutDefaultTemplate' (NameError)

      component_class.define_method(:"_call_#{safe_class_name}", component_class.instance_method(:call))
                                                                                ^^^^^^^^^^^^^^^^
Did you mean?  caller
	from /Users/sigra/projects/view_component/lib/view_component/compiler.rb:110:in `define_render_template_for'
	from /Users/sigra/projects/view_component/lib/view_component/compiler.rb:86:in `compile'
	from /Users/sigra/projects/view_component/lib/view_component/base.rb:577:in `compile'
	from /Users/sigra/projects/view_component/lib/view_component/engine.rb:76:in `each'
	from /Users/sigra/projects/view_component/lib/view_component/engine.rb:76:in `block (2 levels) in <class:Engine>'

@reeganviljoen
Copy link
Collaborator

reeganviljoen commented Jan 10, 2024

@sigra this is a big help thank you

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

3 participants