From ebddc88b988dcf91d248cc75b6bd7c976785bd53 Mon Sep 17 00:00:00 2001 From: Brian Knoles Date: Mon, 21 Oct 2024 18:08:51 -0500 Subject: [PATCH 1/6] Add spec showing that inertia_share accepts a hash --- .../app/controllers/inertia_share_test_controller.rb | 1 + spec/inertia/sharing_spec.rb | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/spec/dummy/app/controllers/inertia_share_test_controller.rb b/spec/dummy/app/controllers/inertia_share_test_controller.rb index 19441db2..1569952d 100644 --- a/spec/dummy/app/controllers/inertia_share_test_controller.rb +++ b/spec/dummy/app/controllers/inertia_share_test_controller.rb @@ -1,6 +1,7 @@ class InertiaShareTestController < ApplicationController inertia_share name: 'Brandon' inertia_share sport: -> { 'hockey' } + inertia_share({a_hash: 'also works'}) inertia_share do { position: 'center', diff --git a/spec/inertia/sharing_spec.rb b/spec/inertia/sharing_spec.rb index 0599ab52..d871b32e 100644 --- a/spec/inertia/sharing_spec.rb +++ b/spec/inertia/sharing_spec.rb @@ -2,7 +2,7 @@ subject { JSON.parse(response.body)['props'].deep_symbolize_keys } context 'using inertia share' do - let(:props) { {name: 'Brandon', sport: 'hockey', position: 'center', number: 29} } + let(:props) { {name: 'Brandon', sport: 'hockey', position: 'center', number: 29, a_hash: 'also works'} } before { get share_path, headers: {'X-Inertia' => true} } it { is_expected.to eq props } @@ -18,7 +18,7 @@ end context 'using inertia share in subsequent requests' do - let(:props) { {name: 'Brandon', sport: 'hockey', position: 'center', number: 29} } + let(:props) { {name: 'Brandon', sport: 'hockey', position: 'center', number: 29, a_hash: 'also works'} } before do get share_path, headers: {'X-Inertia' => true} @@ -29,7 +29,7 @@ end context 'using inertia share with inheritance' do - let(:props) { {name: 'No Longer Brandon', sport: 'hockey', position: 'center', number: 29} } + let(:props) { {name: 'No Longer Brandon', sport: 'hockey', position: 'center', number: 29, a_hash: 'also works'} } before do get share_with_inherited_path, headers: {'X-Inertia' => true} @@ -39,7 +39,7 @@ end context 'with errors' do - let(:props) { {name: 'Brandon', sport: 'hockey', position: 'center', number: 29} } + let(:props) { {name: 'Brandon', sport: 'hockey', position: 'center', number: 29, a_hash: 'also works'} } let(:errors) { 'rearview mirror is present' } before { allow_any_instance_of(ActionDispatch::Request).to receive(:session) { From 93a003435e86dc7f6054d43812e2262fb82d8916 Mon Sep 17 00:00:00 2001 From: Brian Knoles Date: Mon, 21 Oct 2024 18:15:53 -0500 Subject: [PATCH 2/6] Add spec to ensure middleware does not prevent default 404 rendering from Rails --- spec/inertia/request_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/inertia/request_spec.rb b/spec/inertia/request_spec.rb index 6fd79350..ce1dff50 100644 --- a/spec/inertia/request_spec.rb +++ b/spec/inertia/request_spec.rb @@ -154,4 +154,12 @@ end end end + + describe 'a non existent route' do + it 'raises a 404 exception' do + expect { + get '/non_existent_route', headers: { 'X-Inertia' => true } + }.to raise_error(ActionController::RoutingError, /No route matches/) + end + end end From e25dc6beaf9db6943fff96fb872e77462cbeeb4b Mon Sep 17 00:00:00 2001 From: Brian Knoles Date: Mon, 21 Oct 2024 19:45:55 -0500 Subject: [PATCH 3/6] Rewrite the conditional sharing specs to document the behavior of frozen shared data --- .../inertia_conditional_sharing_controller.rb | 14 +++++++ spec/dummy/config/routes.rb | 1 + spec/inertia/conditional_sharing_spec.rb | 39 +++++++++---------- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/spec/dummy/app/controllers/inertia_conditional_sharing_controller.rb b/spec/dummy/app/controllers/inertia_conditional_sharing_controller.rb index 16ab5f6b..3c3d6a38 100644 --- a/spec/dummy/app/controllers/inertia_conditional_sharing_controller.rb +++ b/spec/dummy/app/controllers/inertia_conditional_sharing_controller.rb @@ -1,4 +1,6 @@ class InertiaConditionalSharingController < ApplicationController + before_action :conditionally_share_a_prop, only: :show_with_a_problem + inertia_share normal_shared_prop: 1 inertia_share do @@ -16,4 +18,16 @@ def show show_only_prop: 1, } end + + def show_with_a_problem + render inertia: 'EmptyTestComponent', props: { + show_only_prop: 1, + } + end + + protected + + def conditionally_share_a_prop + self.class.inertia_share incorrectly_conditionally_shared_prop: 1 + end end diff --git a/spec/dummy/config/routes.rb b/spec/dummy/config/routes.rb index 8faf2f91..a45bf49f 100644 --- a/spec/dummy/config/routes.rb +++ b/spec/dummy/config/routes.rb @@ -50,4 +50,5 @@ get 'conditional_share_index' => 'inertia_conditional_sharing#index' get 'conditional_share_show' => 'inertia_conditional_sharing#show' + get 'conditional_share_show_with_a_problem' => 'inertia_conditional_sharing#show_with_a_problem' end diff --git a/spec/inertia/conditional_sharing_spec.rb b/spec/inertia/conditional_sharing_spec.rb index 7f81196c..0453bf8a 100644 --- a/spec/inertia/conditional_sharing_spec.rb +++ b/spec/inertia/conditional_sharing_spec.rb @@ -1,28 +1,27 @@ +# Specs as documentation. Per-action shared data isn't explicity supported, +# but it can be done by referencing the action name in an inertia_share block. RSpec.describe "conditionally shared data in a controller", type: :request do - context "when there is conditional data inside inertia_share" do - it "does not 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} + context "when there is data inside inertia_share only applicable to a single action" do + it "does not leak the data between requests" do 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, - }) + 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, - }) + expect(JSON.parse(response.body)['props'].deep_symbolize_keys).not_to include({ + conditionally_shared_show_prop: 1, + }) + end + end + + context "when there is conditional data shared via before_action" do + it "raises an error because it is frozen" do + expect { + get conditional_share_show_with_a_problem_path, headers: {'X-Inertia' => true} + }.to raise_error(FrozenError) end end end From 4d1de655893d5ddf9b173ef37fa3e5df65c7d2e9 Mon Sep 17 00:00:00 2001 From: Brian Knoles Date: Sat, 26 Oct 2024 18:10:54 -0500 Subject: [PATCH 4/6] Brain Knoles isn't a real person --- inertia_rails.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inertia_rails.gemspec b/inertia_rails.gemspec index b9c082dc..9ea9cc10 100644 --- a/inertia_rails.gemspec +++ b/inertia_rails.gemspec @@ -6,7 +6,7 @@ Gem::Specification.new do |spec| spec.name = "inertia_rails" spec.version = InertiaRails::VERSION spec.authors = ["Brian Knoles", "Brandon Shar", "Eugene Granovsky"] - spec.email = ["brain@bellawatt.com", "brandon@bellawatt.com", "eugene@bellawatt.com"] + spec.email = ["brian@bellawatt.com", "brandon@bellawatt.com", "eugene@bellawatt.com"] spec.summary = %q{Inertia adapter for Rails} spec.homepage = "https://github.com/inertiajs/inertia-rails" From 4453e8f079c0f0bb29dfa7dd1e18d5e5a75828c1 Mon Sep 17 00:00:00 2001 From: Brian Knoles Date: Sat, 26 Oct 2024 18:12:51 -0500 Subject: [PATCH 5/6] Make sure specs pass with Rails < 7.1 --- spec/dummy/config/environments/test.rb | 2 +- spec/inertia/conditional_sharing_spec.rb | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/spec/dummy/config/environments/test.rb b/spec/dummy/config/environments/test.rb index 9efcceb5..adb89080 100644 --- a/spec/dummy/config/environments/test.rb +++ b/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 = :none + config.action_dispatch.show_exceptions = false # Disable request forgery protection in test environment. config.action_controller.allow_forgery_protection = false diff --git a/spec/inertia/conditional_sharing_spec.rb b/spec/inertia/conditional_sharing_spec.rb index 0453bf8a..db0e104f 100644 --- a/spec/inertia/conditional_sharing_spec.rb +++ b/spec/inertia/conditional_sharing_spec.rb @@ -19,6 +19,11 @@ context "when there is conditional data shared via before_action" do it "raises an error because it is frozen" do + # Rails < 7.1 won't raise the error unless we load the controller before the request we actually want to test. + # + # This can be removed when we drop support for Rails < 7.1. + get conditional_share_show_path, headers: {'X-Inertia' => true} + expect { get conditional_share_show_with_a_problem_path, headers: {'X-Inertia' => true} }.to raise_error(FrozenError) From a887fcc0c63c58ec24921c22b1cce63fadb2e2d5 Mon Sep 17 00:00:00 2001 From: Brian Knoles Date: Sat, 26 Oct 2024 19:22:34 -0500 Subject: [PATCH 6/6] Actually, this isn't a Rails version issue. This commit improves the explanation --- spec/inertia/conditional_sharing_spec.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/spec/inertia/conditional_sharing_spec.rb b/spec/inertia/conditional_sharing_spec.rb index db0e104f..05657f10 100644 --- a/spec/inertia/conditional_sharing_spec.rb +++ b/spec/inertia/conditional_sharing_spec.rb @@ -19,10 +19,8 @@ context "when there is conditional data shared via before_action" do it "raises an error because it is frozen" do - # Rails < 7.1 won't raise the error unless we load the controller before the request we actually want to test. - # - # This can be removed when we drop support for Rails < 7.1. - get conditional_share_show_path, headers: {'X-Inertia' => true} + # InertiaSharedData isn't frozen until after the first time it's accessed. + InertiaConditionalSharingController.send(:_inertia_shared_data) expect { get conditional_share_show_with_a_problem_path, headers: {'X-Inertia' => true}