Skip to content

Add strict helpers #1987

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/concerns/view_component/preview_actions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def prepend_application_view_paths
end

def prepend_preview_examples_view_path
prepend_view_path(ViewComponent::Base.preview_paths)
prepend_view_path(ViewComponent::Base.config.preview_paths)
end
end
end
2 changes: 1 addition & 1 deletion app/helpers/preview_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,6 @@ def prism_language_name_by_template_path(template_file_path:)
# :nocov:

def serve_static_preview_assets?
ViewComponent::Base.config.show_previews && Rails.application.config.public_file_server.enabled
ViewComponent::Base.config..show_previews && Rails.application.config.public_file_server.enabled
end
end
16 changes: 10 additions & 6 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,15 @@ nav_order: 6

## main

* Add `helpers_enabled` config.

*Reegan Viljoen*

* Introduce component-local config and migrate `strip_trailing_whitespace` to use it under the hood.

*Simon Fish*
*Simon Fish*

* Add docs about Slack channel in Ruby Central workspace. (Join us! #oss-view-component). Email [email protected] for an invite.
* Add docs about Slack channel in Ruby Central workspace. (Join us! #oss-view-component). Email <[email protected]> for an invite.

*Joel Hawksley

Expand Down Expand Up @@ -1470,7 +1474,7 @@ Run into an issue with this release? [Let us know](https://github.com/ViewCompon

*Joel Hawksley*

* The ViewComponent team at GitHub is hiring! We're looking for a Rails engineer with accessibility experience: [https://boards.greenhouse.io/github/jobs/4020166](https://boards.greenhouse.io/github/jobs/4020166). Reach out to [email protected] with any questions!
* The ViewComponent team at GitHub is hiring! We're looking for a Rails engineer with accessibility experience: [https://boards.greenhouse.io/github/jobs/4020166](https://boards.greenhouse.io/github/jobs/4020166). Reach out to <[email protected]> with any questions!

* The ViewComponent team is hosting a happy hour at RailsConf. Join us for snacks, drinks, and stickers: [https://www.eventbrite.com/e/viewcomponent-happy-hour-tickets-304168585427](https://www.eventbrite.com/e/viewcomponent-happy-hour-tickets-304168585427)

Expand Down Expand Up @@ -2234,7 +2238,7 @@ Run into an issue with this release? [Let us know](https://github.com/ViewCompon

*Matheus Richard*

* Are you interested in building the future of ViewComponent? GitHub is looking to hire a Senior Engineer to work on Primer ViewComponents and ViewComponent. Apply here: [US/Canada](https://github.com/careers) / [Europe](https://boards.greenhouse.io/github/jobs/3132294). Feel free to reach out to [email protected] with any questions.
* Are you interested in building the future of ViewComponent? GitHub is looking to hire a Senior Engineer to work on Primer ViewComponents and ViewComponent. Apply here: [US/Canada](https://github.com/careers) / [Europe](https://boards.greenhouse.io/github/jobs/3132294). Feel free to reach out to <[email protected]> with any questions.

*Joel Hawksley*

Expand All @@ -2252,7 +2256,7 @@ Run into an issue with this release? [Let us know](https://github.com/ViewCompon

## 2.31.0

_Note: This release includes an underlying change to Slots that may affect incorrect usage of the API, where Slots were set on a line prefixed by `<%=`. The result of setting a Slot shouldn't be returned. (`<%`)_
*Note: This release includes an underlying change to Slots that may affect incorrect usage of the API, where Slots were set on a line prefixed by `<%=`. The result of setting a Slot shouldn't be returned. (`<%`)*

* Add `#with_content` to allow setting content without a block.

Expand Down Expand Up @@ -2700,7 +2704,7 @@ _Note: This release includes an underlying change to Slots that may affect incor

* The gem name is now `view_component`.
* ViewComponent previews are now accessed at `/rails/view_components`.
* ViewComponents can _only_ be rendered with the instance syntax: `render(MyComponent.new)`. Support for all other syntaxes has been removed.
* ViewComponents can *only* be rendered with the instance syntax: `render(MyComponent.new)`. Support for all other syntaxes has been removed.
* ActiveModel::Validations have been removed. ViewComponent generators no longer include validations.
* In Rails 6.1, no monkey patching is used.
* `to_component_class` has been removed.
Expand Down
8 changes: 4 additions & 4 deletions lib/rails/generators/abstract_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def file_name
end

def component_path
ViewComponent::Base.config.view_component_path
GlobalConfig.view_component_path
end

def stimulus_controller
Expand All @@ -42,15 +42,15 @@ def stimulus_controller
end

def sidecar?
options["sidecar"] || ViewComponent::Base.config.generate.sidecar
options["sidecar"] || GlobalConfig.generate.sidecar
end

def stimulus?
options["stimulus"] || ViewComponent::Base.config.generate.stimulus_controller
options["stimulus"] || GlobalConfig.generate.stimulus_controller
end

def typescript?
options["typescript"] || ViewComponent::Base.config.generate.typescript
options["typescript"] || GlobalConfig.generate.typescript
end
end
end
6 changes: 3 additions & 3 deletions lib/rails/generators/component/component_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ class ComponentGenerator < Rails::Generators::NamedBase
check_class_collision suffix: "Component"

class_option :inline, type: :boolean, default: false
class_option :locale, type: :boolean, default: ViewComponent::Base.config.generate.locale
class_option :locale, type: :boolean, default: ViewComponent::GlobalConfig.generate.locale
class_option :parent, type: :string, desc: "The parent class for the generated component"
class_option :preview, type: :boolean, default: ViewComponent::Base.config.generate.preview
class_option :preview, type: :boolean, default: ViewComponent::GlobalConfig.generate.preview
class_option :sidecar, type: :boolean, default: false
class_option :stimulus, type: :boolean,
default: ViewComponent::Base.config.generate.stimulus_controller
Expand All @@ -42,7 +42,7 @@ def create_component_file
def parent_class
return options[:parent] if options[:parent]

ViewComponent::Base.config.component_parent_class || default_parent_class
ViewComponent::GlobalConfig.component_parent_class || default_parent_class
end

def initialize_signature
Expand Down
2 changes: 1 addition & 1 deletion lib/rails/generators/locale/component_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class ComponentGenerator < ::Rails::Generators::NamedBase
class_option :sidecar, type: :boolean, default: false

def create_locale_file
if ViewComponent::Base.config.generate.distinct_locale_files
if ViewComponent::GlobalConfig.generate.distinct_locale_files
I18n.available_locales.each do |locale|
create_file destination(locale), translations_hash([locale]).to_yaml
end
Expand Down
4 changes: 2 additions & 2 deletions lib/rails/generators/preview/component_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ module Preview
module Generators
class ComponentGenerator < ::Rails::Generators::NamedBase
source_root File.expand_path("templates", __dir__)
class_option :preview_path, type: :string, desc: "Path for previews, required when multiple preview paths are configured", default: ViewComponent::Base.config.generate.preview_path
class_option :preview_path, type: :string, desc: "Path for previews, required when multiple preview paths are configured", default: ViewComponent::GlobalConfig.generate.preview_path

argument :attributes, type: :array, default: [], banner: "attribute"
check_class_collision suffix: "ComponentPreview"

def create_preview_file
preview_paths = ViewComponent::Base.config.preview_paths
preview_paths = ViewComponent::GlobalConfig.preview_paths
optional_path = options[:preview_path]
return if preview_paths.count > 1 && optional_path.blank?

Expand Down
2 changes: 1 addition & 1 deletion lib/rails/generators/rspec/component_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def create_test_file
private

def spec_component_path
return "spec/components" unless ViewComponent::Base.config.generate.use_component_path_for_rspec_tests
return "spec/components" unless ViewComponent::GlobalConfig.generate.use_component_path_for_rspec_tests

configured_component_path = component_path
if configured_component_path.start_with?("app#{File::SEPARATOR}")
Expand Down
1 change: 1 addition & 0 deletions lib/view_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ module ViewComponent
autoload :ComponentError
autoload :Config
autoload :Deprecation
autoload :GlobalConfig
autoload :InlineTemplate
autoload :Instrumentation
autoload :Preview
Expand Down
22 changes: 15 additions & 7 deletions lib/view_component/base.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# frozen_string_literal: true

require "action_view"
require "active_support/configurable"
require "view_component/collection"
require "view_component/compile_cache"
require "view_component/compiler"
Expand Down Expand Up @@ -44,11 +43,8 @@ def config
RESERVED_PARAMETER = :content
VC_INTERNAL_DEFAULT_FORMAT = :html

# For CSRF authenticity tokens in forms
delegate :form_authenticity_token, :protect_against_forgery?, :config, to: :helpers

# For Content Security Policy nonces
delegate :content_security_policy_nonce, to: :helpers
# For CSRF authenticity tokens in forms and Content Security Policy nonces
use_helpers :form_authenticity_token, :protect_against_forgery?, :config, :content_security_policy_nonce

attr_accessor :__vc_original_view_context

Expand Down Expand Up @@ -232,6 +228,7 @@ def controller
def helpers
raise HelpersCalledBeforeRenderError if view_context.nil?

raise StrictHelperError if !GlobalConfig.helpers_enabled || component_config.strict_helpers_enabled
# Attempt to re-use the original view_context passed to the first
# component rendered in the rendering pipeline. This prevents the
# instantiation of a new view_context via `controller.view_context` which
Expand All @@ -248,7 +245,8 @@ def method_missing(method_name, *args) # rubocop:disable Style/MissingRespondToM
super
rescue => e # rubocop:disable Style/RescueStandardError
e.set_backtrace e.backtrace.tap(&:shift)
raise e, <<~MESSAGE.chomp if view_context && e.is_a?(NameError) && helpers.respond_to?(method_name)
if ViewComponent::Base.strict_helpers_enabled
raise e, <<~MESSAGE.chomp if view_context && e.is_a?(NameError) && (__vc_original_view_context.respond_to?(method_name)|| controller.view_context.respond_to?(method_name))
#{e.message}

You may be trying to call a method provided as a view helper. Did you mean `helpers.#{method_name}`?
Expand Down Expand Up @@ -522,6 +520,16 @@ def inherited(child)
# `compile` defines
compile

if child.superclass == ViewComponent::Base
child.define_singleton_method(:component_config) do
@@component_config ||= Rails.application.config.view_component.component_defaults.inheritable_copy
end
else
child.define_singleton_method(:component_config) do
@@component_config ||= superclass.component_config.inheritable_copy
end
end

# Give the child its own personal #render_template_for to protect against the case when
# eager loading is disabled and the parent component is rendered before the child. In
# such a scenario, the parent will override ViewComponent::Base#render_template_for,
Expand Down
13 changes: 11 additions & 2 deletions lib/view_component/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class << self
alias_method :default, :new

def defaults
ActiveSupport::OrderedOptions.new.merge!({
ActiveSupport::InheritableOptions.new({
generate: default_generate_options,
preview_controller: "ViewComponentsController",
preview_route: "/rails/view_components",
Expand All @@ -25,7 +25,11 @@ def defaults
preview_paths: default_preview_paths,
test_controller: "ApplicationController",
default_preview_layout: nil,
capture_compatibility_patch_enabled: false
capture_compatibility_patch_enabled: false,
helpers_enabled: true,
component_defaults: ActiveSupport::InheritableOptions.new({
strict_helpers_enabled: false
})
})
end

Expand Down Expand Up @@ -173,6 +177,11 @@ def defaults
# previews.
# Defaults to `false`.

# @!attribute helpers_enabled
# @return [Boolean]
# Enables the use of #helpers
# Defaults to `True`.

def default_preview_paths
(default_rails_preview_paths + default_rails_engines_preview_paths).uniq
end
Expand Down
4 changes: 2 additions & 2 deletions lib/view_component/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

module ViewComponent
class Engine < Rails::Engine # :nodoc:
config.view_component = ViewComponent::Config.current
config.view_component = ViewComponent::Config.defaults

if Rails.version.to_f < 8.0
rake_tasks do
Expand All @@ -29,7 +29,7 @@ class Engine < Rails::Engine # :nodoc:
initializer "view_component.set_configs" do |app|
options = app.config.view_component

%i[generate preview_controller preview_route show_previews_source].each do |config_option|
%i[generate preview_controller preview_route].each do |config_option|
options[config_option] ||= ViewComponent::Base.public_send(config_option)
end
options.instrumentation_enabled = false if options.instrumentation_enabled.nil?
Expand Down
4 changes: 4 additions & 0 deletions lib/view_component/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,10 @@ class SystemTestControllerNefariousPathError < BaseError
MESSAGE = "ViewComponent SystemTest controller attempted to load a file outside of the expected directory."
end

class StrictHelperError < BaseError
MESSAGE = "ViewComponent strict helper mode is enbaled so #helpers is disabled."
end

class AlreadyDefinedPolymorphicSlotSetterError < StandardError
MESSAGE =
"A method called 'SETTER_METHOD_NAME' already exists and would be overwritten by the 'SETTER_NAME' polymorphic " \
Expand Down
3 changes: 3 additions & 0 deletions lib/view_component/global_config.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module ViewComponent
GlobalConfig = (defined?(Rails) && Rails.application) ? Rails.application.config.view_component : Base.config # standard:disable Naming/ConstantName
end
2 changes: 1 addition & 1 deletion lib/view_component/instrumentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def render_in(view_context, &block)
private

def notification_name
return "!render.view_component" if ViewComponent::Base.config.use_deprecated_instrumentation_name
return "!render.view_component" if GlobalConfig.use_deprecated_instrumentation_name

"render.view_component"
end
Expand Down
2 changes: 1 addition & 1 deletion lib/view_component/preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def load_previews
private

def preview_paths
Base.preview_paths
ViewComponent::GlobalConfig.preview_paths
end
end
end
Expand Down
3 changes: 2 additions & 1 deletion lib/view_component/test_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,8 @@ def with_request_url(full_path, host: nil, method: nil, format: ViewComponent::B
#
# @return [ActionController::Base]
def vc_test_controller
@vc_test_controller ||= __vc_test_helpers_build_controller(Base.test_controller.constantize)
config_source = defined?(Rails) ? Rails.application.config.view_component : ViewComponent::Base
@vc_test_controller ||= __vc_test_helpers_build_controller(config_source.test_controller.constantize)
end

# Access the request used by `render_inline`:
Expand Down
4 changes: 3 additions & 1 deletion lib/view_component/use_helpers.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true

require "view_component/errors"

module ViewComponent::UseHelpers
extend ActiveSupport::Concern

Expand All @@ -13,7 +15,7 @@ def use_helper(helper_method, from: nil, prefix: false)

class_eval(<<-RUBY, __FILE__, __LINE__ + 1)
def #{helper_method_name}(*args, &block)
raise HelpersCalledBeforeRenderError if view_context.nil?
raise ::ViewComponent::HelpersCalledBeforeRenderError if view_context.nil?

#{define_helper(helper_method: helper_method, source: from)}
end
Expand Down
12 changes: 12 additions & 0 deletions test/sandbox/test/base_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,18 @@ def test_no_method_error_does_not_reference_helper_if_view_context_not_present
assert !exception_message_regex.match?(exception.message)
end

def test_no_heleprs_error_if_helpers_disabled
with_helpers_enabled_config(false) do
exception = assert_raises(NoMethodError) { Class.new(ViewComponent::Base).new.current_user }
exception_message_regex = Regexp.new <<~MESSAGE.chomp, Regexp::MULTILINE
undefined method `current_user' for .*

You may be trying to call a method provided as a view helper. To use it try decalring it using use_helpers :current_user'?
MESSAGE
assert !exception_message_regex.match?(exception.message)
end
end

def test_no_method_error_references_helper_if_view_context_present
view_context = ActionController::Base.new.view_context
view_context.instance_eval {
Expand Down
6 changes: 3 additions & 3 deletions test/sandbox/test/integration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ def test_render_component_in_turbo_stream
expected_response_body = <<~TURBOSTREAM
<turbo-stream action="update" target="area1"><template><span>Hello, world!</span></template></turbo-stream>
TURBOSTREAM
if ViewComponent::Base.config.capture_compatibility_patch_enabled
if ViewComponent::GlobalConfig.capture_compatibility_patch_enabled
assert_equal expected_response_body, response.body
else
assert_not_equal expected_response_body, response.body
Expand Down Expand Up @@ -716,8 +716,8 @@ def test_cached_partial
Rails.cache.clear
end

def test_config_options_shared_between_base_and_engine
config_entrypoints = [Rails.application.config.view_component, ViewComponent::Base.config]
def test_globalconfig_is_proxy_for_rails_app_config
config_entrypoints = [Rails.application.config.view_component, ViewComponent::GlobalConfig]
2.times do
config_entrypoints.first.yield_self do |config|
{
Expand Down
Loading
Loading