diff --git a/lib/inertia_rails.rb b/lib/inertia_rails.rb index ef6b0885..b8ea0f49 100644 --- a/lib/inertia_rails.rb +++ b/lib/inertia_rails.rb @@ -21,8 +21,15 @@ module InertiaRails class Error < StandardError; end + class UnoptimizedPartialReloadError < StandardError; end def self.deprecator # :nodoc: @deprecator ||= ActiveSupport::Deprecation.new end + + def self.warn(message) + full_message = "[InertiaRails]: WARNING! #{message}" + Kernel.warn full_message if Rails.env.development? || Rails.env.test? + Rails.logger.warn full_message + end end diff --git a/lib/inertia_rails/configuration.rb b/lib/inertia_rails/configuration.rb index 02f73f05..df1a13dd 100644 --- a/lib/inertia_rails/configuration.rb +++ b/lib/inertia_rails/configuration.rb @@ -22,6 +22,8 @@ class Configuration # Used to detect version drift between server and client. version: nil, + + raise_on_unoptimized_partial_reloads: false, }.freeze OPTION_NAMES = DEFAULTS.keys.freeze diff --git a/lib/inertia_rails/renderer.rb b/lib/inertia_rails/renderer.rb index ec090c26..6fc329c8 100644 --- a/lib/inertia_rails/renderer.rb +++ b/lib/inertia_rails/renderer.rb @@ -68,7 +68,10 @@ def merge_props(shared_data, props) end def computed_props - _props = merge_props(shared_data, props).select do |key, prop| + _merged_props = merge_props(shared_data, props) + validate_partial_props(_merged_props) + + _filtered_props = _merged_props.select do |key, prop| if rendering_partial_component? key.in? partial_keys else @@ -77,7 +80,7 @@ def computed_props end deep_transform_values( - _props, + _filtered_props, lambda do |prop| prop.respond_to?(:call) ? controller.instance_exec(&prop) : prop end @@ -112,5 +115,32 @@ def resolve_component(component) configuration.component_path_resolver(path: controller.controller_path, action: controller.action_name) end + + def validate_partial_props(merged_props) + return unless rendering_partial_component? + + unrequested_props = merged_props.select do |key, prop| + !key.in?(partial_keys) + end + + props_that_will_unecessarily_compute = unrequested_props.select do |key, prop| + !prop.respond_to?(:call) + end + + if props_that_will_unecessarily_compute.present? + props = props_that_will_unecessarily_compute.keys.map { |k| ":#{k}" }.join(', ') + is_plural = props_that_will_unecessarily_compute.length > 1 + verb = is_plural ? "props are" : "prop is" + pronoun = is_plural ? "them because they are defined as values" : "it because it is defined as a value" + + warning_message = "The #{props} #{verb} being computed even though your partial reload did not request #{pronoun}. You might want to wrap these in a callable like a lambda ->{} or InertiaRails::Lazy()." + + if configuration.raise_on_unoptimized_partial_reloads + raise InertiaRails::UnoptimizedPartialReloadError, warning_message + else + InertiaRails.warn warning_message + end + end + end end end diff --git a/spec/dummy/app/controllers/concerns/searchable.rb b/spec/dummy/app/controllers/concerns/searchable.rb new file mode 100644 index 00000000..61f5b0e4 --- /dev/null +++ b/spec/dummy/app/controllers/concerns/searchable.rb @@ -0,0 +1,13 @@ +module Searchable + extend ActiveSupport::Concern + + included do + inertia_config(raise_on_unoptimized_partial_reloads: true) + + inertia_share do + { + search: { query: '', results: [] } + } + end + end +end diff --git a/spec/dummy/app/controllers/inertia_has_searchable_controller.rb b/spec/dummy/app/controllers/inertia_has_searchable_controller.rb new file mode 100644 index 00000000..793ba678 --- /dev/null +++ b/spec/dummy/app/controllers/inertia_has_searchable_controller.rb @@ -0,0 +1,9 @@ +class InertiaHasSearchableController < ApplicationController + include Searchable + + def index + render inertia: 'TestComponent', props: { + unrequested_prop: 'This will always compute even when not requested in a partial reload', + } + end +end diff --git a/spec/dummy/app/controllers/inertia_unoptimized_partial_reloads_controller.rb b/spec/dummy/app/controllers/inertia_unoptimized_partial_reloads_controller.rb new file mode 100644 index 00000000..32eaaf64 --- /dev/null +++ b/spec/dummy/app/controllers/inertia_unoptimized_partial_reloads_controller.rb @@ -0,0 +1,21 @@ +class InertiaUnoptimizedPartialReloadsController < ApplicationController + inertia_share search: { query: '', results: [] } + + def index + render inertia: 'TestComponent', props: { + expensive_prop: expensive_prop, + } + end + + def with_multiple + render inertia: 'TestComponent', props: { + expensive_prop: expensive_prop, + another_expensive_prop: "another one", + callable_prop: -> { 'This is a callable prop' }, + } + end + + def expensive_prop + 'Imagine this is slow to compute' + end +end diff --git a/spec/dummy/config/routes.rb b/spec/dummy/config/routes.rb index 42b8fb3f..fcb87dcb 100644 --- a/spec/dummy/config/routes.rb +++ b/spec/dummy/config/routes.rb @@ -56,4 +56,9 @@ get 'conditional_share_show' => 'inertia_conditional_sharing#show' get 'conditional_share_edit' => 'inertia_conditional_sharing#edit' get 'conditional_share_show_with_a_problem' => 'inertia_conditional_sharing#show_with_a_problem' + + get 'unoptimized_partial_reloads' => 'inertia_unoptimized_partial_reloads#index' + get 'unoptimized_partial_reloads_with_exception' => 'inertia_unoptimized_partial_reloads#with_exception' + get 'unoptimized_partial_reloads_with_mutiple' => 'inertia_unoptimized_partial_reloads#with_multiple' + get 'has_searchable' => 'inertia_has_searchable#index' end diff --git a/spec/inertia/partial_reload_spec.rb b/spec/inertia/partial_reload_spec.rb new file mode 100644 index 00000000..52b659e4 --- /dev/null +++ b/spec/inertia/partial_reload_spec.rb @@ -0,0 +1,88 @@ +RSpec.describe 'partial reloads', type: :request do + describe 'optimization guard rails' do + context 'when a non-requested prop is defined as a value' do + let (:fake_std_err) {FakeStdErr.new} + let (:warn_message) {'[InertiaRails]: WARNING! The :expensive_prop prop is being computed even though your partial reload did not request it because it is defined as a value. You might want to wrap these in a callable like a lambda ->{} or InertiaRails::Lazy().'} + let (:warn_message_with_multiple) {'[InertiaRails]: WARNING! The :expensive_prop, :another_expensive_prop props are being computed even though your partial reload did not request them because they are defined as values. You might want to wrap these in a callable like a lambda ->{} or InertiaRails::Lazy().'} + + around(:each) do |example| + begin + original_stderr = $stderr + $stderr = fake_std_err + + example.run + ensure + $std_err = original_stderr + end + end + + it 'only returns the requested prop' do + get unoptimized_partial_reloads_path, headers: { + 'X-Inertia' => true, + 'X-Inertia-Partial-Data' => 'search', + 'X-Inertia-Partial-Component' => 'TestComponent', + } + + expect(JSON.parse(response.body)['props'].deep_symbolize_keys).to eq({ + search: { + query: '', + results: [], + }, + }) + end + + it 'computes the non-requested prop anyway' do + expect_any_instance_of(InertiaUnoptimizedPartialReloadsController).to receive(:expensive_prop).with(any_args) + + get unoptimized_partial_reloads_path, headers: { + 'X-Inertia' => true, + 'X-Inertia-Partial-Data' => 'search', + 'X-Inertia-Partial-Component' => 'TestComponent', + } + end + + it 'emits a warning' do + get unoptimized_partial_reloads_path, headers: { + 'X-Inertia' => true, + 'X-Inertia-Partial-Data' => 'search', + 'X-Inertia-Partial-Component' => 'TestComponent', + } + expect(fake_std_err.messages[0].chomp).to(eq(warn_message)) + end + + it 'does not warn about callable props' do + get unoptimized_partial_reloads_path, headers: { + 'X-Inertia' => true, + 'X-Inertia-Partial-Data' => 'search', + 'X-Inertia-Partial-Component' => 'TestComponent', + } + + expect(fake_std_err.messages[0].chomp).not_to include('callable_prop') + end + + context 'when there are multiple non-requested props defined as values' do + it 'emits a different warning' do + get unoptimized_partial_reloads_with_mutiple_path, headers: { + 'X-Inertia' => true, + 'X-Inertia-Partial-Data' => 'search', + 'X-Inertia-Partial-Component' => 'TestComponent', + } + + expect(fake_std_err.messages[0].chomp).to(eq(warn_message_with_multiple)) + end + end + + context 'when the controller is configured to raise_on_unoptimized_partial_reloads' do + it 'raises an error' do + expect { + get has_searchable_path, headers: { + 'X-Inertia' => true, + 'X-Inertia-Partial-Data' => 'search', + 'X-Inertia-Partial-Component' => 'TestComponent', + } + }.to raise_error(InertiaRails::UnoptimizedPartialReloadError, /unrequested_prop/) + end + end + end + end +end diff --git a/spec/inertia/rspec_helper_spec.rb b/spec/inertia/rspec_helper_spec.rb index 12a6fb37..c122c358 100644 --- a/spec/inertia/rspec_helper_spec.rb +++ b/spec/inertia/rspec_helper_spec.rb @@ -1,21 +1,5 @@ require_relative '../../lib/inertia_rails/rspec' -class FakeStdErr - attr_accessor :messages - - def initialize - @messages = [] - end - - def write(msg) - @messages << msg - end - - # Rails 5.0 + Ruby 2.6 require puts to be a public method - def puts(thing) - end -end - RSpec.describe InertiaRails::RSpec, type: :request do describe 'correctly set up inertia tests with inertia: true', inertia: true do context 'with props' do diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index f89646ee..edf7b707 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -13,6 +13,8 @@ Dir[Rails.root.join('spec', 'support', '**', '*.rb')].each { |f| require f } require_relative './support/helper_module' +require_relative './support/fake_std_error' + # Add additional requires below this line. Rails is not loaded until this point! # Requires supporting ruby files with custom matchers and macros, etc, in # spec/support/ and its subdirectories. Files matching `spec/**/*_spec.rb` are diff --git a/spec/support/fake_std_error.rb b/spec/support/fake_std_error.rb new file mode 100644 index 00000000..e9f1f7ee --- /dev/null +++ b/spec/support/fake_std_error.rb @@ -0,0 +1,15 @@ +class FakeStdErr + attr_accessor :messages + + def initialize + @messages = [] + end + + def write(msg) + @messages << msg + end + + # Rails 5.0 + Ruby 2.6 require puts to be a public method + def puts(thing) + end +end