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

Add slot methods to a module instead of the component class itself #2040

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

ozydingo
Copy link

@ozydingo ozydingo commented Jun 1, 2024

What are you trying to accomplish?

Addressed #2027:

  • Define slot methods in a dynamic module so that slot name methods can be overridden with ability to call super

What approach did you choose and why?

  • Inspired by ActiveRecord's naming of GeneratedAttributeMethods, I am choosing the name GeneratedSlotMethods
  • Define a new module and set it to the const self::GeneratedSlotMethods in the inherited hook defined by ViewComponent::Slotable, similar to how ActiveRecord creates <model>::GeneratedAttributeMethods.
  • include self::GeneratedSlotMethods in the same inherited hook
  • define the <slot> and <slot>? methods on the module

TODO

  • FIX: Use the inherited hook and not the included hook; ViewComponent::Slotable is included only once by ViewComponent::Base
  • Add test to make sure independent components do not share methods
  • Ensure component subclasses that define their own slots also inherit superclass slot methods
  • Investigate other slottable dynamic methods (currently commented out)

Anything you want to highlight for special attention from reviewers?

  • Added a step in CONTRIBUTING to bundle install, with the goal of installing appraisal (otherwise bundle exec appraisal install would fail)
  • Running bundle exec appraisal install modified the gemfiles and I'm not sure why. I have no experience with appraisal. I imagine I will need to revert these changes, but I'm keeping them on the branch for now since if I revert I can't run tests, and also to ask about it.

@ozydingo ozydingo changed the title Ahs generated methods Add slot methods to a module instead of the component class itself Jun 1, 2024
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

Successfully merging this pull request may close these issues.

None yet

1 participant