-
Notifications
You must be signed in to change notification settings - Fork 468
Add after_compile hook for component extensions #2411
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
base: main
Are you sure you want to change the base?
Add after_compile hook for component extensions #2411
Conversation
7aee501
to
a0eeb0f
Compare
lib/view_component/compiler.rb
Outdated
return if compiled? && !force | ||
|
||
gather_templates | ||
ActiveSupport::Notifications.instrument( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After thinking about how we might write the documentation for this feature, my gut tells me that we might be better served with something simpler.
What if we instead added a no-op public after_compile
method to Base
and call it inside https://github.com/ViewComponent/view_component/blob/main/lib/view_component/base.rb#L539
? That way, consumers could define their own after_compile
that takes additional actions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The after_compile
hook seems much cleaner and avoids the Rails 7.1 issues. I’ll update the PR to use that approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps using ActiveSupport::Callbacks would make it easier to inject multiple extensions into the base gem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joelhawksley would you be open to exploring that direction, or do you prefer to keep it minimal for now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer the minimal approach. I don't think we need any more complexity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joelhawksley I've updated the PR to use the minimal after_compile approach as you suggested
909437c
to
889e015
Compare
889e015
to
8ed08c8
Compare
Following up on the conversation with @joelhawksley and @fsateler in this comment, this PR introduces a minimal extension point in the
ViewComponent::Compiler
.What are you trying to accomplish?
This PR creates a minimal extension point for
ViewComponent::Compiler
to enable "sub-templates" functionality through a dedicated gem. The sub-templates gem will work by overriding the after_compile hook, receiving the compiled component, and then searching for additional template files (e.g., header.html.erb, row.html.erb) in the component's directory to generate correspondingcall_*
methods with explicit locals validation.What approach did you choose and why?
I chose to implement a simple after_compile instance method hook as suggested by @joelhawksley. This approach is:
Simple and Direct: Following Joel's preference for simplicity over sophisticated callback systems
Easy to Document: A straightforward method override pattern that's familiar to Rails developers
Zero Overhead: No performance impact when the hook isn't overridden
Non-invasive: Doesn't modify ViewComponent's core compilation logic
Extensible: Components can easily override the method to add custom post-compilation behavior
The implementation uses allocate to call the instance method without triggering the component's constructor, avoiding the complexity of handling initialization arguments.
Anything you want to highlight for special attention from reviewers?
@component.allocate.after_compile
to avoid constructor complexity, but I'm uncertain if this is the most elegant solution. Open to suggestions for alternative approaches that don't requireallocate
.