Consider Removing `use_inertia_instance_props`

[BrandonShar]

I am currently not convinced we should remove this functionality. As I said in #239, I view this feature as a sharp knife and believe it's a nice opt-in for folks who understand what they're getting into. BUT, I do understand the objection and want to make sure I'm not dismissing it too quickly. So this issue is to understand community sentiment on this feature. If you have an opinion, please add it below!

As @skryukov said in the original PR #239 :

While use_inertia_instance_props is a clever way to mimic Rails behavior, it also exposes too much data on the frontend side, which is very dangerous. Therefore, I propose deprecating it.

And @bknoles replied:

Ok, I'm following now. So, I'd say something like this isn't super risky and is a sharp knives situation:

class MyController
  before_action :check_something_expensive
  use_inertia_instance_props
  
  def index
    @other_thing = OtherThing.find(params[:id])

    render inertia: 'OtherThing/Show'
  end
  
  # non-Inertia action
  def other_action
    # Reason for memoizing
    render json: my_expensive_thing
  end

  protected

  def check_something_expensive
    raise 'Uh oh' if my_expensive_thing.broken?
  end

  def my_expensive_thing
    @_my_expensive_thing ||= ExpensiveCalc.new.calculate
  end
end

However this one would worry me:

class MyController < ApplicationController
  include AwesomeLLMHelperFromGem
  use_inertia_instance_props

  def index
    @thing = Thing.find(params[:id])

    render inertia: 'Things/Show'
  end
end

# Inside gem
module AwesomeLLMHelperFromGem
  included do
    before_action do
      @_openai_token = OpenAI.authenticate(some_stuff)
    end
  end

  ...
end 

That one would (probably suprisingly) ship credentials right into the Inertia response. As I look through the code, @_inertia_skip_props seems intended to mitigate the risk, but it's going to depend on the order that the before_actions are registered / order of operations, etc.

[bknoles]

What about changing the API to be an allow list?

class MyController < ApplicationController
  use_inertia_instance_props :thing, :other_thing

  def first_action
    @thing = Thing.find(params[:thing_id])

    render inertia: 'Things/FirstAction'
  end

  def second_action
    @other_thing = OtherThing.find(params[:other_thing_id])

    render inertia: 'OtherThings/SecondAction'
  end

  def third_action
    # This would NOT be passed to the client as a prop
    thing = Thing.find(params[:thing_id])
    thing.do_something

    render inertia: 'SomeOtherPage'
  end
end

It's less elegant, but still a bit cleaner than the default prop passing for those who love to squeeze code out of their controllers. And it removes the risk of accidentally exposing something to the client.

(This thought inspired by the decent_exposure library that a teammate sent my way)

[skryukov]

I don't see the point in this vs explicit 'render inertia: foo:, bar:'

[bknoles]

I think it's just a stylistic preference. The hypothetical user of use_inertia_instance_props (which I am not) doesn't mind explicitly passing a page path but prefers not to have to manually craft a props object. Or maybe they also use default_render so that their controller actions look even leaner?

class MyController < ApplicationController
  # probably in a base controller or initializer
  inertia_config(default_render: true)

  use_inertia_instance_props :thing

  def show
    @thing = Thing.find(params[:id])
  end
end

(That would theoretically work, right?)

Personally, I prefer to explicitly render both the page path and the props object. All these renderer tricks are too much magic for my taste but I don't mind accommodating the crazy people reasonable-people-who-disagree-with-me that like them 😁.