Delegate config to parent modules (no-op) #2231
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds a mechanism for inheriting config from a parent module. There's definitely more that needs to be done to iron this out, including: - if the module does not already respond to `config`, provide a concern that sets it up using `ActiveSupport::Configurable` - if the module _does_ already respond to `config` and doesn't have `view_component` set on it, set it up. - if the module's config does have `view_component` set up, leave it alone? I also need to consider whether the current way of having things refer back to the Rails app config is appropriate. Since `Base` is generally used when referring to "global" config options, this probably doesn't do anything right now, but settings like `test_controller` and `generate` should opt into it.
This can be included on a module to provide module-local configuration for ViewComponent. It avoids overwriting existing configuration that may already exist.
boardfish
commented
Mar 17, 2025
Comment on lines
+151
to
+191
| module TestModuleWithoutConfig | ||
| class SomeComponent < ViewComponent::Base | ||
| end | ||
| end | ||
|
|
||
| # Config defined on top-level module as opposed to engine. | ||
| module TestModuleWithConfig | ||
| include ViewComponent::Configurable | ||
|
|
||
| configure do |config| | ||
| config.view_component.test_controller = "AnotherController" | ||
| end | ||
|
|
||
| class SomeComponent < ViewComponent::Base | ||
| end | ||
| end | ||
|
|
||
| module TestAlreadyConfigurableModule | ||
| include ActiveSupport::Configurable | ||
| include ViewComponent::Configurable | ||
|
|
||
| configure do |config| | ||
| config.view_component.test_controller = "AnotherController" | ||
| end | ||
|
|
||
| class SomeComponent < ViewComponent::Base | ||
| end | ||
| end | ||
|
|
||
| module TestAlreadyConfiguredModule | ||
| include ActiveSupport::Configurable | ||
|
|
||
| configure do |config| | ||
| config.view_component = ActiveSupport::InheritableOptions[test_controller: "AnotherController"] | ||
| end | ||
|
|
||
| include ViewComponent::Configurable | ||
|
|
||
| class SomeComponent < ViewComponent::Base | ||
| end | ||
| end |
There was a problem hiding this comment.
Happy to move this setup out into other files if you'd rather.
boardfish
changed the title
joelhawksley
approved these changes
Mar 18, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a mechanism for inheriting config from a parent module. Since
Baseis generally used when referring to "global" config options, this doesn't do anything right now beyond exposing the ability to configure parent modules.Modules can be configured by requiring and including
ViewComponent::Configurable, which will expose aconfigaccessor with aview_componentconfiguration on the module. These will not interfere with existing configuration that might be granted to, e.g., aRails::Engineby default.This would give us a starting point to begin migrating options into engine-local config.
The separation between:
lib/view_component/engine.rb)strip_trailing_whitespaceto use component-local config under the hood #2230)isn't perfect right now, but this PR and #2230 put us on the road to this.
It should only be possible to set options in each area as appropriate. Particularly with this PR, we should be concerned with making sure that module-local and exclusively Rails app-local config don't merge.
Because
ViewComponent::Base.configis named as it is right now, the difference between component-local config (#2230) and module-local config isn't super clear, but we can rename parts of this interface in v4.