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

Stimulus reflex concern #464

Merged

Conversation

julianrubisch
Copy link
Contributor

@julianrubisch julianrubisch commented Mar 4, 2021

Enhancement

Description

This PR adds a ConcernEnhancer module that can be included alongside an ActiveSupport::Concern (on which's existence it depends), producing a concern that can be used by both reflex and controller (and models, for that matter) without having to guard against undefined methods (like helper_method, before_action etc.).

It basically adds a method_missing class method to the includer, which will stub class methods for the counterpart after switching on the ancestor (whether it's a StimulusReflex::Reflex, ActionController::Base etc.)

NOTE for docs: this module has to be included as opposed to ActiveSupport::Concern, which has to be extended:

module TestConcern
  extend ActiveSupport::Concern
  include StimulusReflex::ConcernEnhancer

  # .. more methods
end

Why should this be added

To more resiliently share code between controllers/models and reflexes

Checklist

  • My code follows the style guidelines of this project
  • Checks (StandardRB & Prettier-Standard) are passing
  • This is not a documentation update

Please note that the best way to suggest changes or updates to the documentation is to join Discord and leave a note in the #docs channel. Any documentation updates posted as PRs cannot be accepted at this time. ❤️

@hopsoft
Copy link
Contributor

hopsoft commented Mar 20, 2021

Can this be updated so usage looks more like this?

module TestConcern
  extend StimulusReflex::SharableConcern
end

@hopsoft
Copy link
Contributor

hopsoft commented Mar 20, 2021

We also need to think about where we recommend that such concerns would exist in the project structure so it can be documented.

app/controllers/concerns
app/reflexes/concerns
app/???

@hopsoft
Copy link
Contributor

hopsoft commented Mar 20, 2021

My preference is app/controllers/concerns as a general rule of thumb, but there will be exceptions.

@julianrubisch julianrubisch force-pushed the stimulus-reflex--concern branch from dea181b to 4485bfe Compare March 30, 2021 07:02
@leastbad leastbad merged commit dd63441 into stimulusreflex:master Mar 30, 2021
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.

3 participants