Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Refactor inertia share to use instance variables #111

Merged
merged 20 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
35d01a0
Rewrite "inertia_share" to drop InertiaRails.reset!
ledermann Jul 18, 2020
c17171a
Test inertia_share with redirects
ledermann Nov 21, 2020
d3f86f4
Add test for exception handler
ledermann Nov 21, 2020
ff6833c
merge with origin
PedroAugustoRamalhoDuarte Apr 11, 2024
4dfe6b5
refactor inertia headers for class variable
PedroAugustoRamalhoDuarte Apr 11, 2024
55293f4
:refactor: rewrite multithread spec
PedroAugustoRamalhoDuarte Apr 12, 2024
ba50ac4
:refactor: refactor InertiaRails::Controller to use only instance var…
PedroAugustoRamalhoDuarte Apr 12, 2024
0158d32
suppress testing warning
PedroAugustoRamalhoDuarte Apr 12, 2024
f0dcd35
make sure inertia initialize shared data for renderer
PedroAugustoRamalhoDuarte Apr 28, 2024
9bbf451
refactor inertia html headers setter
PedroAugustoRamalhoDuarte Apr 28, 2024
6160d38
cleanup test files
PedroAugustoRamalhoDuarte Apr 28, 2024
5da531a
Merge remote-tracking branch 'other/master' into fix-data-sharing
PedroAugustoRamalhoDuarte May 28, 2024
4349026
removes old ::InertiaRails.reset! from rspec
PedroAugustoRamalhoDuarte May 28, 2024
7c3152a
call shared_data directly without send method
PedroAugustoRamalhoDuarte May 28, 2024
077f5ad
conditionally shared data specs
bknoles Apr 28, 2024
b7d69cc
Mark as todo (an admittedly contrived) failing spec for conditionally…
PedroAugustoRamalhoDuarte May 28, 2024
a306898
improves conditional_sharing_spec.rb for edge case documentation
PedroAugustoRamalhoDuarte Jun 1, 2024
6037973
rollback old multithreaded testing
PedroAugustoRamalhoDuarte Jun 1, 2024
a9cb801
removes unused methods from InertiaRails::Controller
PedroAugustoRamalhoDuarte Jun 1, 2024
c622c7a
refactor shared data method
PedroAugustoRamalhoDuarte Jun 1, 2024
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
40 changes: 33 additions & 7 deletions lib/inertia_rails/controller.rb
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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
7 changes: 0 additions & 7 deletions lib/inertia_rails/helper.rb

This file was deleted.

54 changes: 0 additions & 54 deletions lib/inertia_rails/inertia_rails.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
5 changes: 1 addition & 4 deletions lib/inertia_rails/middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ def initialize(app)
def call(env)
InertiaRailsRequest.new(@app, env)
.response
ensure
::InertiaRails.reset!
end

class InertiaRailsRequest
Expand All @@ -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)
Expand Down Expand Up @@ -97,4 +95,3 @@ def copy_xsrf_to_csrf!
end
end
end

8 changes: 3 additions & 5 deletions lib/inertia_rails/renderer.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
require 'net/http'
require 'json'
require_relative "inertia_rails"

module InertiaRails
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion lib/inertia_rails/rspec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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
10 changes: 8 additions & 2 deletions spec/dummy/app/controllers/inertia_share_test_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions spec/dummy/app/controllers/inertia_test_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 0 additions & 1 deletion spec/dummy/config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,3 @@ class Application < Rails::Application
config.secret_key_base = SecureRandom.hex
end
end

2 changes: 1 addition & 1 deletion spec/dummy/config/environments/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions spec/dummy/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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
29 changes: 29 additions & 0 deletions spec/inertia/conditional_sharing_spec.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion spec/inertia/rendering_spec.rb
Original file line number Diff line number Diff line change
@@ -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) }
Expand Down
Loading