stimulusreflex / stimulus_reflex

Build reactive applications with the Rails tooling you already know and love.
https://docs.stimulusreflex.com
MIT License
2.28k stars 172 forks source link

ActionController Parameters #199

Closed jasoncharnes closed 4 years ago

jasoncharnes commented 4 years ago

Feature Request

The ability to access "params" inside a reflex.

Is your feature request related to a problem?

I have a large form that has two requirements:

  1. It needs to autosave the entire form after a field is changed
    • This includes misc reflexes that may do something unrelated to just persisting, but still needs to persist the form as a side-effect
  2. It should go through the normal Rails validation process when saving
    • If it fails, I want to keep all the fields in the form on re-render (keep the bad data, show validations, keep instantiated nested has many, etc.)

Given the size of the form, and the requirements listed, I'd like to send all the form fields to Stimulus Reflex. I can do this using arguments, like so:

<% # app/views/posts/edit.html.erb %>
<%= form_with model: @post, data: { controller: "posts" } do |form| %>
  <%= form.hidden_field :id %>
  <%= form.text_field :name, data: { action: "change->posts#submitPost" } %>
<% end %>
// app/javascript/controllers/posts_controller.js
get params() {
  // Using https://www.npmjs.com/package/form-serialize
  return serialize(this.element, { hash: true, empty: true });
}

submitPost(event) {
  this.stimulate("PostReflex#submit_post", event.target, this.params);
}
# app/reflexes/post_reflex.rb
class PostReflex < ApplicationReflex
  def submit_post(params = {})
    @post = Post.find(params.dig('post', 'id')
    @post.assign_attributes(params.dig('post'))
    @post.save
    true # to always trigger a re-render
  end
end

However, after building the page, we end up with multiple reflexes that handle different separate, but similar requirements.

Without assigning the post from the attributes each time, we would lose access to the form fields and could potentially (and our case, likely) trigger a re-render without the persisted data.

A benefit to this approach is that for a "small amount" of code, and by sending the attributes each time, when we attempt to autosave we implicitly get #validate. When StimulusReflex re-renders we have error messages and css for free, and we didn't lose data! 🎉

But, this means we need to repeat the 1. find, 2. assign, 3. save process for each reflex. I'd like to extract that into before and after callbacks but, to my knowledge, we can't access arguments in callbacks.

@hopsoft showed me how he handles sending a form with parameters to a reflex, stores the attributes in the session, and re-instantiates the model with the params. (It's really cool!)

Unfortunately, we use cookie store. While it's on the docket to potentially change to a database session store in the future, we're not there yet. That means, we don't have access to the session. We also don't want to use Rails cache, because we don't want to lose the data due to eviction (we have a pretty large cache in terms of available size, but also in terms of how much it's used).

This leaves us with a fair amount of duplication for sending form data and attempting to persist:

Describe the solution you'd like

The ability to send params to a Reflex and have it available as an attr_reader inside the reflex so it's available in callbacks.

Here's how our reflex looks (code changed to "posts") at work after implementing params in our fork of StimulusReflex:

<% # app/views/posts/edit.html.erb %>
<%= form_with model: @post, data: { controller: "posts" } do |form| %>
  <%= form.hidden_field :id %>
  <%= form.text_field :name, data: { action: "change->posts#submitPost" } %>
  <%= form.fields_for :categories, @post.categories do |category_form| %>
    ... form for adding a new category while creating a new post
  <% end 
<% end %>
// app/javascript/controllers/posts_controller.js

// Sent implicitly with `this.stimulate` as to not override the existing argument functionality
get params() {
  // Using https://www.npmjs.com/package/form-serialize
  return serialize(this.element, { hash: true, empty: true });
}

submitPost(event) {
  this.stimulate("PostReflex#submit_post", event.target);
}
# app/reflexes/post_reflex.rb
class PostReflex < ApplicationReflex
  before_reflex -> { @post = Post.find(params.dig(:post, :id)) }
  before_reflex -> { @post.assign_attributes(post_params) }
  after_reflex -> { @post.save; true }, except: :build_new_category

  # In this case, all our functionality is handled by callbacks
  def submit_post; end

  # If using nested attributes, the field will be rendered when reflex re-renders
  # By passing params each time, we don't have to keep "state" of categories
  # because params is our "source of truth" and also handles validations.
  def build_new_category
    @post.categories.build
  end

  private

  def post_params
    params.require(:post).permit(:name, categories_attributes: [:id, :name])
  end
end

What I like about this approach:

What I don't like about this approach:

NOTESSSSSS

I'm still new to StimulusReflex, so I may be missing the bigger picture. This is the result of a 40-hour week of thinking about my given problem.

I'd love to hear from others on:

  1. How they'd solve this
  2. If they could/would use this proposal

Sorry for the big issue. I wanted to explain in detail what I was looking at/considering. I'm very appreciative for anyone taking the time to read this and for all of you who have contributed to make StimulusReflex awesome. ❤️

CC @excid3 @basilkhan05 @ideasasylum

leastbad commented 4 years ago

What a wild thing to try to parse on a phone. 🤤

Seriously though, it’s amazing that you took the time to detail your thoughts.

I suspect Nate said this already but this sounds like a job for UJS and the Optimism gem. You might be using SR like a hammer - it wasn’t really designed for forms.

I have a hare-brained scheme in mind on my todo list for a kind of intermediate bucket storage object, designed to hold model attributes while you’re making a series of updates that individually might leave the model in an invalid state if you’re expecting to hit save at the end. (Think: a new model instance with multiple validates presence: true attrs). This currently imaginary library would make SR much more applicable to form context instead of just action context. However, there’s no timeline.

UJS+Optimism; potent new version of Optimism in master, likely cutting a release tomorrow.

hopsoft commented 4 years ago

Thanks for the thoughtful issue @jasoncharnes. It's very helpful and does bring up a valid scenario where we could simplify form state management without too much additional work. I'll dig into your implementation and maybe we can pair this week sometime to discuss further.

jasoncharnes commented 4 years ago

What a wild thing to try to parse on a phone.

As I was typing this, I was thinking, "they're going to hate me." 😅

You might be using SR like a hammer - it wasn’t really designed for forms.

I was afraid of this. I was struggling to find the line and wasn't sure if I was going in the wrong direction.

@leastbad I'll take a deeper look at Optimism this week and see if it's a better solution to my problem. ❤️


I'll dig into your implementation and maybe we can pair this week sometime to discuss further.

@hopsoft I'd love to pair! My implementation is very much a POC and could use some more thought, so I'm glad to discuss it further.

Thank you both for taking the time to read this and reply. 🤗

jasoncharnes commented 4 years ago

I had an idea for a simpler solution that would allow me to use my params approach, without having to shove the concept of params into StimulusReflex.

If I could access arguments in my callbacks, I could convert arguments into params.

this.stimulate('PostReflex#do_thing', this.element, code_to_serialize_form_into_hash);
class PostReflex < ApplicationReflex
  attr_reader :params
  before_action -> { @params = ActionController::Parameters.new(arguments.first) }
end

Having access to the arguments in the callbacks may also be useful outside of my use case, as well.

What I'm doing with my callbacks in the example is a very "sharp knives" approach because my arguments might be more than a single hash I want to turn to params, but that's something I could work out on my implementation versus making Reflex figure that out.

I sketched this out this morning.

Just a thought. 🤗

leastbad commented 4 years ago

I didn't realize that arguments weren't available in callbacks... but I suppose that makes sense. I didn't implement the callbacks system but I feel like surely we should be able to solve this at the library level.

"surely" is always easier when you're not the doer.

But yeah, just in case there's any confusion: the arguments are definitely passed into the reflex action method on a 1:1 basis.

vzctl commented 4 years ago

Hey!

I have a similar use case with management of large forms and autosaving (and I also have a Post model ;)

Here's how I implemented it: https://gist.github.com/vzctl/9707a6092dab41c873a3113d02e2245c

Here are some choices I made:

1) Stimulus-managed forms need to be compatible with normal remote forms in rails. Depending on the flow I can define if the form action is performed using StimulusReflex or UJS.

2) Minimize cognitive overhead. Because of that I wanted to keep business logic in regular controllers and keep reflex classes slim and predictable.

It's my first time working with StimulusReflex and I felt like I needed to make quite a few decisions when it comes to forms management.

Hope it's helpful!