Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

cable_car aka 'Ajax mode' #108

Merged
merged 12 commits into from
Mar 30, 2021
Merged

Conversation

leastbad
Copy link
Contributor

@leastbad leastbad commented Feb 7, 2021

People have started to notice that you can call CableReady.perform anywhere, including outside of an ActionCable#received callback. @jonsgreen calls it inside of a custom CR operation, transforming it into a DSL for DOM manipulation. All you need is a viable JSON payload and you're good to go.

As we prepare for a multi-transport future in SR, so too must CR adapt to non-ActionCable usage cases. This PR proposes cable_car: a simplified, transport-agnostic class used for generating CableReady-compliant JSON payloads.

cable_car has an almost identical usage pattern as string identifier channels inside Reflex classes, except that instead of broadcast, chains are terminated by .dispatch.

cable_car is a Singleton like cable_ready, and chains the same operations list from the config class. Calling dispatch clears out the enqueued operations unless you call it with clear: false. It should be completely fine to use cable_ready and cable_car alongside each other simultaneously.

Thanks to @joshleblanc, the Channel and CableCar are both subclasses that inherit from a new common superclass called OperationBuilder. This means that there is no duplicated code and further changes to OperationBuilder will benefit both.

I also removed what I believe were vestigal references to @operations from the Channels class.

Rails.application.routes.draw do
  get "fetch", to: "home#fetch", constraints: lambda { |request| request.xhr? } 
end
import { Controller } from 'stimulus'
import CableReady from 'cable_ready'

export default class extends Controller {
  fetch () {
    fetch('/fetch.json', {
      headers: { 'X-Requested-With': 'XMLHttpRequest' }
    })
      .then(response => response.json())
      .then(data => CableReady.perform(data))
  }
}
<button data-controller="cable-car" data-action="cable-car#fetch">Cable Car</button>
<div id="users"></div>
class HomeController < ApplicationController
  include CableReady::Broadcaster

  def fetch
    render operations: cable_car
      .inner_html(selector: "#users", html: "<span>winning</span>")
      .set_focus(selector: "#users")
      .play_sound(src: "song.mp3")
      .dispatch
  end
end
# {"innerHtml":[{"selector":"#users","html":"\u003cspan\u003ewinning\u003c/span\u003e"}],"setFocus":[{"selector":"#users"}],"playSound":[{"src":"song.mp3"}]}

Serialized operations via the apply! method

There needs to be a simple way to turn serialized operations into enqueued operations.

The apply! concept was lifted from @julianrubisch's cable_ready_callbacks gem, (where it was originally apply).

Keep in mind that while dispatch returns a Hash, apply! will accept both a Hash or a JSON string.

operations = cable_car.inner_html(selector: "#users", html: "<b>Users</b>").dispatch # enqueued_operations is now empty
operations # {"innerHtml"=>[{"selector"=>"#users", "html"=>"<b>Users</b>"}]}
cable_car.apply!(operations) # @enqueued_operations={"innerHtml"=>[{"selector"=>"#users", "html"=>"<b>Users</b>"}]}>
cable_car.dispatch # cleared out queue
cable_car.apply!(operations.to_json) # @enqueued_operations={"innerHtml"=>[{"selector"=>"#users", "html"=>"<b>Users</b>"}]}>

I also made it so that both channels and cable_car now have a to_json that returns the JSON for enqueued_operations instead of the whole object, and this, too, can be passed into apply!.

Custom ActionController Renderer

Once again thanks to @joshleblanc, controllers can now respond with a CableReady JSON payload, ready to be passed into CableReady.perform().

class HomeController < ApplicationController
  def index
    render operations: cable_car.console_log(message: "hi")
  end
end

@leastbad leastbad added enhancement proposal ruby Pull requests that update Ruby code labels Feb 7, 2021
@leastbad leastbad marked this pull request as draft February 7, 2021 07:21
@jaredcwhite
Copy link
Contributor

jaredcwhite commented Mar 18, 2021

@leastbad Coming into this with very little context (looks like a killer API so far!) — if render json: cable_car ... will implicitly call to_json, you wouldn't actually need an action verb like ride, would you? The to_json call itself would convert from the DSL's internal state to a hash. I do think ride is fun wordplay there, but ideally something that could simply be abstracted out via Rails magic.

update: ah, I just noticed you did have to_json at one point and changed it. I'm guessing there's a good reason behind that…

@leastbad
Copy link
Contributor Author

Thanks! I'm just building on what was already here, and borrowing good ideas from @julianrubisch.

The short answer is that the reason (I think) a terminating verb is necessary is that a render action is not the only place you might call this. In fact, the whole impetus behind doing it is to rework the backend so that there's a consistent queueing API and then loads of syntactic sugar (like the implicit render stuff) to keep up our BMW 7 Series vibe.

The other wildcard is that @joshleblanc made a PR on this PR which I haven't had a chance to review, but my spidey sense tells me is going to have a major impact on what's already been done.

@leastbad leastbad marked this pull request as ready for review March 23, 2021 01:25
@hopsoft
Copy link
Contributor

hopsoft commented Mar 27, 2021

This is exciting. I love the wordplay but would like to propose different semantics.

operations = cable_car.inner_html(...).console_log(...).enqueued_operations
cable_car.dispatch # vs "ride", flushes enqueued_operations
cable_car.append(operations) # vs "apply", appends operations to enqueued_operations (alt option: enqueue)

@hopsoft
Copy link
Contributor

hopsoft commented Mar 27, 2021

The new renderer also opens the door for ActionCable based responses. What do you think about this proposal?

ActionController::Renderers.add :cable_car do
  render json: cable_car.dispatch
end

ActionController::Renderers.add :cable_ready do
  cable_ready.broadcast
  head :ok
end
class HomeController < ApplicationController
  def index
    cable_car.console_log(message: "hi")
    render :cable_car
  end

  def show
    cable_ready.console_log(message: "hi")
    render :cable_ready
  end 
end

That's not a complete example, but it communicates the idea. Let's go for the land grab and add support for a cable_ready renderer on this PR too.

@julianrubisch
Copy link
Contributor

Exciting! I haven't watched closely but am thrilled to read all the docs afterwards 😜

@leastbad
Copy link
Contributor Author

You're not proposing that we make people put enqueued_operations at the end of a chain, right?

I prefer dispatch to ride, nice call. It even works well with the theme. I'll change this around once we sort out the rest.

append is actually already an operation. enqueue is challenging for a lot of folks to spell. What about ingest?

I think that your render :cable_car concept looks awesome. I guess we'd do that instead of the render operations: or would we support both?

I don't totally understand what you're proposing with render :cable_ready though. How would you specify a channel identifier? How would you specify a resource if the channel is a stream_for? It might help if you explained how you saw people using this form.

@hopsoft
Copy link
Contributor

hopsoft commented Mar 27, 2021

You're not proposing that we make people put enqueued_operations at the end of a chain, right?

Correct. This is already supported (but not required). I was just using it as an example of how to get a reference to some operations.

append is actually already an operation. enqueue is challenging for a lot of folks to spell. What about ingest?

Good catch on append. What do you think about merge!?

cable_car.merge! operations

Let me expand on the renderers API a bit and see if we can land on something that everyone likes. The CableReady renderers are more challenging because CR is so incredibly flexible.

cable_car

# multi line example
def index
  cable_car.console_log(message: "hi")
  cable_car.console_log(message: "hi again")
  render :cable_car
end

# single line example
def index
  render cable_car: cable_car.console_log(message: "hi")
end

cable_ready

def index
  render broadcast: cable_ready['string-identifier'].console_log(message: "hello world ")
end

def index
  render broadcast: [
    cable_ready['one'].console_log(message: "hello world from one"),
    cable_ready['two'].console_log(message: "hello world from two")
  ]
end

def index
  render broadcast_to: Helen.find(30), cable_ready[HelensChannel].console_log(message: "hello helen")
end

def index
  render broadcast_to: Helen.find(30), [
    cable_ready[HelensChannel1].console_log(message: "hello helen from 1"),
    cable_ready[HelensChannel2].console_log(message: "hello helen from 2")
  ]
end

@leastbad
Copy link
Contributor Author

I don't love merge! but I do like the !. I was going to suggest it earlier but actually removed the comment. :)

Anything jump out at you here? accept!, recreate!, regenerate!, restore!, revive!

https://www.merriam-webster.com/thesaurus/ingest
https://www.merriam-webster.com/thesaurus/queue
https://www.merriam-webster.com/thesaurus/load
https://www.merriam-webster.com/thesaurus/absorb

Naming is super hard.


The cable_car operation is really interesting. The "single line" example has the same syntax as what's currently in the PR, just renamed from operation: to cable_car:, although your implementation would support multi-line and not require any parameters to the method.

While I'm ultimately fine with switching from operation to cable_car, I would point out that when we were discussing the naming of what ultimately became the "nothing morph" it was because "nothing" is what you're morphing. I wonder if through this lens, the original render operations: is more faithful to your naming philosophy? After all, you're not rendering a "cable_car", you're rendering a chain of operations. Let me know what you think is best.


I understand the "what" for a cable_ready renderer, but I'm still fuzzy on the why. It would really help me if you could describe some scenarios where this would be useful, with bonus points for context about what existing technique this could replace.

From an implementation perspective, I agree that introducing a broadcast and broadcast_to is the way to go.

I would advise putting the resource after the cable_ready call so that it's consistent with the existing experience.

I like the single-line forms (1 and 3, above) but the array-based multiple channel approach makes me hate the way 2 and 4 look. I might be biased because I still don't understand what this is for, but it just seems like one too-many permutations of syntax for my train to track effectively. 🧠

I feel like the same naming lens critique applies to render broadcast: as does render cable_car: in that in the context of an HTTP request/response, you're not really rendering a broadcast. You're rendering a head :ok and sending something else, somewhere else. To me, that's confusing and defies what I think of when I render something today.

I don't want to sound negative! I do want to carefully weigh every new code path for benefits vs the perceived complexity and decision fatigue of the library.

@joshleblanc
Copy link
Contributor

standardrb is automatically converting

    @operation_builder.merge!(foobar: [{name: "passed_option"}])

to

    @operation_builder[:foobar] = [{name: "passed_option"}]

Which is definitely not what we want. Should we consider this a standardrb bug, or look at different names?

@joshleblanc
Copy link
Contributor

Actually, maybe we should just get operation builder to respond to [] and []=. That would solve our problem and make it behave like one might expect?

Copy link
Contributor

@hopsoft hopsoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great.

@leastbad leastbad merged commit 7a07544 into stimulusreflex:master Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement proposal ruby Pull requests that update Ruby code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants