Partial reload guard rails

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

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

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

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

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

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

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

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

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

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

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