Unoptimized partial reload warnings

lib/inertia_rails.rb

@@ -21,6 +21,14 @@
 
 module InertiaRails
   class Error < StandardError; end
+  class UnoptimizedPartialReload < StandardError
+    attr_reader :paths
+
+    def initialize(paths)
+      @paths = paths
+      super("The #{paths.join(', ')} prop(s) were excluded in a partial reload but still evaluated because they are defined as values.")
+    end
+  end
 
   def self.deprecator # :nodoc:
     @deprecator ||= ActiveSupport::Deprecation.new

lib/inertia_rails/configuration.rb

@@ -22,6 +22,8 @@ class Configuration
 
       # Used to detect version drift between server and client.
       version: nil,
+
+      action_on_unoptimized_partial_reload: :log,
     }.freeze
 
     OPTION_NAMES = DEFAULTS.keys.freeze

lib/inertia_rails/engine.rb

@@ -12,5 +12,20 @@ class Engine < ::Rails::Engine
         include ::InertiaRails::Controller
       end
     end
+
+    initializer "inertia_rails.subscribe_to_notifications" do
+      if Rails.env.development? || Rails.env.test?
+        ActiveSupport::Notifications.subscribe('inertia_rails.unoptimized_partial_render') do |*args|
+          event = ActiveSupport::Notifications::Event.new(*args)
+
+          message =
+            "InertiaRails: The \"#{event.payload[:paths].join(', ')}\" " \
+          "prop(s) were excluded in a partial reload but still evaluated because they are defined as values. " \
+          "Consider wrapping them in something callable like a lambda."
+
+          Rails.logger.debug(message)
+        end
+      end
+    end
   end
 end

lib/inertia_rails/renderer.rb

@@ -6,9 +6,6 @@
 
 module InertiaRails
   class Renderer
-    KEEP_PROP = :keep
-    DONT_KEEP_PROP = :dont_keep
-
     attr_reader(
       :component,
       :configuration,
@@ -30,21 +27,30 @@ def initialize(component, controller, request, response, render_method, props: n
     end
 
     def render
+      set_vary_header
+
+      validate_partial_reload_optimization if rendering_partial_component?
+
+      return render_inertia_response if @request.headers['X-Inertia']
+      return render_ssr if configuration.ssr_enabled rescue nil
+
+      render_full_page
+    end
+
+    private
+
+    def set_vary_header
       if @response.headers["Vary"].blank?
         @response.headers["Vary"] = 'X-Inertia'
       else
         @response.headers["Vary"] = "#{@response.headers["Vary"]}, X-Inertia"
       end
-      if @request.headers['X-Inertia']
-        @response.set_header('X-Inertia', 'true')
-        @render_method.call json: page, status: @response.status, content_type: Mime[:json]
-      else
-        return render_ssr if configuration.ssr_enabled rescue nil
-        @render_method.call template: 'inertia', layout: layout, locals: view_data.merge(page: page)
-      end
     end
 
-    private
+    def render_inertia_response
+      @response.set_header('X-Inertia', 'true')
+      @render_method.call json: page, status: @response.status, content_type: Mime[:json]
+    end
 
     def render_ssr
       uri = URI("#{configuration.ssr_url}/render")
@@ -54,6 +60,10 @@ def render_ssr
       @render_method.call html: res['body'].html_safe, layout: layout, locals: view_data.merge(page: page)
     end
 
+    def render_full_page
+      @render_method.call template: 'inertia', layout: layout, locals: view_data.merge(page: page)
+    end
+
     def layout
       layout = configuration.layout
       layout.nil? ? true : layout
@@ -68,58 +78,28 @@ def shared_data
     #
     # Functionally, this permits using either string or symbol keys in the controller. Since the results
     # is cast to json, we should treat string/symbol keys as identical.
-    def merge_props(shared_data, props)
-      if @deep_merge
-        shared_data.deep_symbolize_keys.deep_merge!(props.deep_symbolize_keys)
+    def merged_props
+      @merged_props ||= if @deep_merge
+        shared_data.deep_symbolize_keys.deep_merge!(@props.deep_symbolize_keys)
       else
-        shared_data.symbolize_keys.merge(props.symbolize_keys)
+        shared_data.symbolize_keys.merge(@props.symbolize_keys)
       end
     end
 
-    def computed_props
-      _props = merge_props(shared_data, props)
-
-      deep_transform_props _props do |prop, path|
-        next [DONT_KEEP_PROP] unless keep_prop?(prop, path)
-
-        transformed_prop = case prop
-        when BaseProp
-          prop.call(controller)
-        when Proc
-          controller.instance_exec(&prop)
-        else
-          prop
-        end
-
-        [KEEP_PROP, transformed_prop]
-      end
+    def prop_transformer
+      @prop_transformer ||= PropTransformer.new(controller)
+        .select_transformed(merged_props){ |prop, path| keep_prop?(prop, path) }
     end
 
     def page
       {
         component: component,
-        props: computed_props,
+        props: prop_transformer.props,
         url: @request.original_fullpath,
         version: configuration.version,
       }
     end
 
-    def deep_transform_props(props, parent_path = [], &block)
-      props.reduce({}) do |transformed_props, (key, prop)|
-        current_path = parent_path + [key]
-
-        if prop.is_a?(Hash) && prop.any?
-          nested = deep_transform_props(prop, current_path, &block)
-          transformed_props.merge!(key => nested) unless nested.empty?
-        else
-          action, transformed_prop = block.call(prop, current_path)
-          transformed_props.merge!(key => transformed_prop) if action == KEEP_PROP
-        end
-
-        transformed_props
-      end
-    end
-
     def partial_keys
       (@request.headers['X-Inertia-Partial-Data'] || '').split(',').compact
     end
@@ -166,5 +146,72 @@ def excluded_by_only_partial_keys?(path_with_prefixes)
     def excluded_by_except_partial_keys?(path_with_prefixes)
       partial_except_keys.present? && (path_with_prefixes & partial_except_keys).any?
     end
+
+    def validate_partial_reload_optimization
+      if prop_transformer.unoptimized_prop_paths.any?
+        case configuration.action_on_unoptimized_partial_reload
+        when :log
+          ActiveSupport::Notifications.instrument(
+            'inertia_rails.unoptimized_partial_render',
+            paths: prop_transformer.unoptimized_prop_paths,
+            controller: controller,
+            action: controller.action_name,
+          )
+        when :raise
+          raise InertiaRails::UnoptimizedPartialReload.new(prop_transformer.unoptimized_prop_paths)
+        end
+      end
+    end
+
+    class PropTransformer
+      attr_reader :props, :unoptimized_prop_paths
+
+      def initialize(controller)
+        @controller = controller
+        @unoptimized_prop_paths = []
+        @props = {}
+      end
+
+      def select_transformed(initial_props, &block)
+        @unoptimized_prop_paths = []
+        @props = deep_transform_and_select(initial_props, &block)
+
+        self
+      end
+
+      private
+
+      def deep_transform_and_select(original_props, parent_path = [], &block)
+        original_props.reduce({}) do |transformed_props, (key, prop)|
+          current_path = parent_path + [key]
+
+          if prop.is_a?(Hash) && prop.any?
+            nested = deep_transform_and_select(prop, current_path, &block)
+            transformed_props.merge!(key => nested) unless nested.empty?
+          elsif block.call(prop, current_path)
+            transformed_props.merge!(key => transform_prop(prop))
+          elsif !prop.respond_to?(:call)
+            track_unoptimized_prop(current_path)
+          end
+
+          transformed_props
+        end
+      end
+
+      def transform_prop(prop)
+        case prop
+        when BaseProp
+          prop.call(@controller)
+        when Proc
+          @controller.instance_exec(&prop)
+        else
+          prop
+        end
+      end
+
+      def track_unoptimized_prop(path)
+        @unoptimized_prop_paths << path.join(".")
+      end
+    end
   end
 end

spec/dummy/config/environments/test.rb

@@ -9,7 +9,7 @@
 
 Rails.application.configure do
   # Settings specified here will take precedence over those in config/application.rb.
-  
+
   config.cache_classes = false
 
   # Do not eager load code on boot. This avoids loading your whole application

spec/inertia/configuration_spec.rb

@@ -153,11 +153,60 @@
       end
     end
   end
+
+  describe '.action_on_unoptimized_partial_reload' do
+    let(:headers) {{
+      'X-Inertia' => true,
+      'X-Inertia-Partial-Data' => 'nested.first',
+      'X-Inertia-Partial-Component' => 'TestComponent',
+    }}
+
+    context 'default case' do
+      let(:headers) {{
+        'X-Inertia' => true,
+        'X-Inertia-Partial-Data' => 'nested.first',
+        'X-Inertia-Partial-Component' => 'TestComponent',
+      }}
+
+      it 'logs a warning' do
+        expect(Rails.logger).to receive(:debug).with(/flat, nested\.second/)
+        get except_props_path, headers: headers
+      end
+    end
+
+    context 'when set to :raise' do
+      before do
+        InertiaRails.configure {|c| c.action_on_unoptimized_partial_reload = :raise}
+      end
+
+      let(:headers) {{
+        'X-Inertia' => true,
+        'X-Inertia-Partial-Data' => 'nested.first',
+        'X-Inertia-Partial-Component' => 'TestComponent',
+      }}
+
+      it 'raises an exception' do
+        expect { get except_props_path, headers: headers }.to raise_error(InertiaRails::UnoptimizedPartialReload).with_message(/flat, nested\.second/)
+      end
+    end
+
+    context 'with an unknown value' do
+      before do
+        InertiaRails.configure {|c| c.action_on_unoptimized_partial_reload = :unknown}
+      end
+
+      it 'does nothing' do
+        expect(Rails.logger).not_to receive(:debug)
+        get except_props_path, headers: headers
+      end
+    end
+  end
 end
 
 def reset_config!
   InertiaRails.configure do |config|
     config.version = nil
     config.layout = 'application'
+    config.action_on_unoptimized_partial_reload = :log
   end
 end