Merge pull request #111 from PedroAugustoRamalhoDuarte/fix-data-sharing

Refactor inertia share to use instance variables

lib/inertia_rails/controller.rb

@@ -1,27 +1,37 @@
 require_relative "inertia_rails"
-require_relative "helper"
 
 module InertiaRails
   module Controller
     extend ActiveSupport::Concern
 
     included do
+      helper_method :inertia_headers
+
       before_action do
-        # :inertia_errors are deleted from the session by the middleware
-        InertiaRails.share(errors: session[:inertia_errors]) if session[:inertia_errors].present?
+        error_sharing = proc do
+          # :inertia_errors are deleted from the session by the middleware
+          if @_request && session[:inertia_errors].present?
+            { errors: session[:inertia_errors] }
+          else
+            {}
+          end
+        end
+
+        @_inertia_shared_plain_data ||= {}
+        @_inertia_shared_blocks ||= [error_sharing]
+        @_inertia_html_headers ||= []
       end
-      helper ::InertiaRails::Helper
 
       after_action do
         cookies['XSRF-TOKEN'] = form_authenticity_token unless !protect_against_forgery?
       end
     end
 
     module ClassMethods
-      def inertia_share(**args, &block)
+      def inertia_share(hash = nil, &block)
         before_action do
-          InertiaRails.share(**args) if args
-          InertiaRails.share_block(block) if block
+          @_inertia_shared_plain_data = @_inertia_shared_plain_data.merge(hash) if hash
+          @_inertia_shared_blocks = @_inertia_shared_blocks + [block] if block_given?
         end
       end
 
@@ -33,6 +43,14 @@ def use_inertia_instance_props
       end
     end
 
+    def inertia_headers
+      @_inertia_html_headers.join.html_safe
+    end
+
+    def inertia_headers=(value)
+      @_inertia_html_headers = value
+    end
+
     def default_render
       if InertiaRails.default_render?
         render(inertia: true)
@@ -41,6 +59,10 @@ def default_render
       end
     end
 
+    def shared_data
+      (@_inertia_shared_plain_data || {}).merge(evaluated_blocks)
+    end
+
     def redirect_to(options = {}, response_options = {})
       capture_inertia_errors(response_options)
       super(options, response_options)
@@ -80,5 +102,9 @@ def capture_inertia_errors(options)
         session[:inertia_errors] = inertia_errors
       end
     end
+
+    def evaluated_blocks
+      (@_inertia_shared_blocks || []).map { |block| instance_exec(&block) }.reduce(&:merge) || {}
+    end
   end
 end

lib/inertia_rails/inertia_rails.rb

@@ -3,20 +3,10 @@
 require 'inertia_rails/lazy'
 
 module InertiaRails
-  thread_mattr_accessor :threadsafe_shared_plain_data
-  thread_mattr_accessor :threadsafe_shared_blocks
-  thread_mattr_accessor :threadsafe_html_headers
-
   def self.configure
     yield(Configuration)
   end
 
-  # "Getters"
-  def self.shared_data(controller)
-    shared_plain_data.
-      merge!(evaluated_blocks(controller, shared_blocks))
-  end
-
   def self.version
     Configuration.evaluated_version
   end
@@ -35,35 +25,12 @@ def self.ssr_url
 
   def self.default_render?
     Configuration.default_render
-  end 
-
-  def self.html_headers
-    self.threadsafe_html_headers || []
   end
 
   def self.deep_merge_shared_data?
     Configuration.deep_merge_shared_data
   end
 
-  # "Setters"
-  def self.share(**args)
-    self.shared_plain_data = self.shared_plain_data.merge(args)
-  end
-
-  def self.share_block(block)
-    self.shared_blocks = self.shared_blocks + [block]
-  end
-
-  def self.html_headers=(headers)
-    self.threadsafe_html_headers = headers
-  end
-
-  def self.reset!
-    self.shared_plain_data = {}
-    self.shared_blocks = []
-    self.html_headers = []
-  end
-
   def self.lazy(value = nil, &block)
     InertiaRails::Lazy.new(value, &block)
   end
@@ -82,25 +49,4 @@ def self.evaluated_version
       self.version.respond_to?(:call) ? self.version.call : self.version
     end
   end
-
-  # Getters and setters to provide default values for the threadsafe attributes
-  def self.shared_plain_data
-    self.threadsafe_shared_plain_data || {}
-  end
-
-  def self.shared_plain_data=(val)
-    self.threadsafe_shared_plain_data = val
-  end
-
-  def self.shared_blocks
-    self.threadsafe_shared_blocks || []
-  end
-
-  def self.shared_blocks=(val)
-    self.threadsafe_shared_blocks = val
-  end
-
-  def self.evaluated_blocks(controller,  blocks)
-    blocks.flat_map { |block| controller.instance_exec(&block) }.reduce(&:merge) || {}
-  end
 end

lib/inertia_rails/middleware.rb

@@ -7,8 +7,6 @@ def initialize(app)
     def call(env)
       InertiaRailsRequest.new(@app, env)
                          .response
-    ensure
-      ::InertiaRails.reset!
     end
 
     class InertiaRailsRequest
@@ -22,7 +20,7 @@ def response
         status, headers, body = @app.call(@env)
         request = ActionDispatch::Request.new(@env)
 
-        # Inertia errors are added to the session via redirect_to 
+        # Inertia errors are added to the session via redirect_to
         request.session.delete(:inertia_errors) unless keep_inertia_errors?(status)
 
         status = 303 if inertia_non_post_redirect?(status)
@@ -97,4 +95,3 @@ def copy_xsrf_to_csrf!
     end
   end
 end
-

lib/inertia_rails/renderer.rb

@@ -1,5 +1,3 @@
-require 'net/http'
-require 'json'
 require_relative "inertia_rails"
 
 module InertiaRails
@@ -33,8 +31,8 @@ def render
     def render_ssr
       uri = URI("#{::InertiaRails.ssr_url}/render")
       res = JSON.parse(Net::HTTP.post(uri, page.to_json, 'Content-Type' => 'application/json').body)
-      
-      ::InertiaRails.html_headers = res['head']
+
+      @controller.inertia_headers = res['head']
       @render_method.call html: res['body'].html_safe, layout: layout, locals: (view_data).merge({page: page})
     end
 
@@ -48,7 +46,7 @@ def computed_props
       #
       # 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.
-      _props = ::InertiaRails.shared_data(@controller).deep_symbolize_keys.send(prop_merge_method, @props.deep_symbolize_keys).select do |key, prop|
+      _props = @controller.shared_data.merge.deep_symbolize_keys.send(prop_merge_method, @props.deep_symbolize_keys).select do |key, prop|
         if rendering_partial_component?
           key.in? partial_keys
         else

lib/inertia_rails/rspec.rb

@@ -65,7 +65,6 @@ def inertia_tests_setup?
   }
 
   config.before(:each, inertia: true) do
-    ::InertiaRails.reset!
     new_renderer = InertiaRails::Renderer.method(:new)
     allow(InertiaRails::Renderer).to receive(:new) do |component, controller, request, response, render, named_args|
       new_renderer.call(component, controller, request, response, inertia_wrap_render(render), **named_args)

spec/dummy/app/controllers/inertia_conditional_sharing_controller.rb

@@ -0,0 +1,22 @@
+class InertiaConditionalSharingController < ApplicationController
+  before_action :conditionally_share_props, only: [:show]
+  inertia_share normal_shared_prop: 1
+
+  def index
+    render inertia: 'EmptyTestComponent', props: {
+      index_only_prop: 1,
+    }
+  end
+
+  def show
+    render inertia: 'EmptyTestComponent', props: {
+      show_only_prop: 1,
+    }
+  end
+
+  protected
+
+  def conditionally_share_props
+    self.class.inertia_share conditionally_shared_show_prop: 1
+  end
+end

spec/dummy/app/controllers/inertia_share_test_controller.rb

@@ -4,11 +4,17 @@ class InertiaShareTestController < ApplicationController
   inertia_share do
     {
       position: 'center',
-      number: 29,
+      number: number,
     }
   end
-  
+
   def share
     render inertia: 'ShareTestComponent'
   end
+
+  private
+
+  def number
+    29
+  end
 end

spec/dummy/app/controllers/inertia_test_controller.rb

@@ -64,4 +64,8 @@ def content_type_test
       format.xml { render xml: [ 1, 2, 3 ] }
     end
   end
+
+  def redirect_to_share_test
+    redirect_to share_path
+  end
 end

spec/dummy/config/application.rb

@@ -34,4 +34,3 @@ class Application < Rails::Application
     config.secret_key_base = SecureRandom.hex
   end
 end
-

spec/dummy/config/environments/test.rb

@@ -29,7 +29,7 @@
   config.cache_store = :null_store
 
   # Raise exceptions instead of rendering exception templates.
-  config.action_dispatch.show_exceptions = false
+  config.action_dispatch.show_exceptions = :none
 
   # Disable request forgery protection in test environment.
   config.action_controller.allow_forgery_protection = false

spec/dummy/config/routes.rb

@@ -33,6 +33,7 @@
   get 'default_component_test' => 'inertia_rails_mimic#default_component_test'
   get 'provided_props_test' => 'inertia_rails_mimic#provided_props_test'
 
+  post 'redirect_to_share_test' => 'inertia_test#redirect_to_share_test'
   inertia 'inertia_route' => 'TestComponent'
 
   get 'merge_shared' => 'inertia_merge_shared#merge_shared'
@@ -45,4 +46,7 @@
   get 'initialize_session' => 'inertia_session_continuity_test#initialize_session'
   post 'submit_form_to_test_csrf' => 'inertia_session_continuity_test#submit_form_to_test_csrf'
   delete 'clear_session' => 'inertia_session_continuity_test#clear_session'
+
+  get 'conditional_share_index' => 'inertia_conditional_sharing#index'
+  get 'conditional_share_show' => 'inertia_conditional_sharing#show'
 end

spec/inertia/conditional_sharing_spec.rb

@@ -0,0 +1,29 @@
+RSpec.describe "conditionally shared data in a controller", type: :request do
+  context "when there is a before_action inside a inertia_share" do
+    it "does leak data between requests" do
+      get conditional_share_index_path, headers: {'X-Inertia' => true}
+      expect(JSON.parse(response.body)['props'].deep_symbolize_keys).to eq({
+                                                                             index_only_prop: 1,
+                                                                             normal_shared_prop: 1,
+                                                                           })
+
+      # NOTE: we actually have to run the show action twice since the new implementation
+      # sets up a before_action within a before_action to share the data.
+      # In effect, that means that the shared data isn't rendered until the second time the action is run.
+      get conditional_share_show_path, headers: {'X-Inertia' => true}
+      get conditional_share_show_path, headers: {'X-Inertia' => true}
+      expect(JSON.parse(response.body)['props'].deep_symbolize_keys).to eq({
+                                                                             normal_shared_prop: 1,
+                                                                             show_only_prop: 1,
+                                                                             conditionally_shared_show_prop: 1,
+                                                                           })
+
+      get conditional_share_index_path, headers: {'X-Inertia' => true}
+      expect(JSON.parse(response.body)['props'].deep_symbolize_keys).to eq({
+                                                                             index_only_prop: 1,
+                                                                             normal_shared_prop: 1,
+                                                                             conditionally_shared_show_prop: 1,
+                                                                           })
+    end
+  end
+end

spec/inertia/rendering_spec.rb

@@ -1,7 +1,7 @@
 RSpec.describe 'rendering inertia views', type: :request do
   subject { response.body }
 
-  let(:controller) { double('Controller', inertia_view_assigns: {})}
+  let(:controller) { ApplicationController.new }
 
   context 'first load' do
     let(:page) { InertiaRails::Renderer.new('TestComponent', controller, request, response, '').send(:page) }