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

Setup a proxy object that wraps CableReady::Channels #382

Merged
merged 3 commits into from
Nov 28, 2020

Conversation

hopsoft
Copy link
Contributor

@hopsoft hopsoft commented Nov 26, 2020

Enhancement

Description

Expose cable_ready to reflexes through a proxy object to improve the developer ergonomics when emitting CableReady broadcasts from within a reflex on the StimulusReflex channel.

Example Usage

class ExampleReflex < ApplicationReflex
  def do_stuff
    # business logic....

    cable_ready
      .push_state(url: "https://example.com/123")
      .dispatch_event(name: "wtf", detail: {message: "Holy shit!"})
      .broadcast

    # default behavior of full page morph still runs
    # you could also opt to call: morph [SELECTOR, :nothing]
  end
end

Note this is the server side counter part to #381

Checklist

  • My code follows the style guidelines of this project
  • Checks (StandardRB & Prettier-Standard) are passing
  • This is not a documentation update

This improves developer ergonimics for emitting CR broadcasts on the SR
channel within a reflex.
@marcoroth
Copy link
Member

Nice, this really improves the developer experience!

@hopsoft
Copy link
Contributor Author

hopsoft commented Nov 26, 2020

We should be careful not to encourage element mutations via CableReady if/when someone is using default reflex behavior.

@leastbad
Copy link
Contributor

I will be addressing this in the docs, 100%.

One thing that might not be self-evident from the readme above is that this also implicitly solves the "how do I send a CableReady broadcast just to the current user" question, at least within a Reflex.

@hopsoft
Copy link
Contributor Author

hopsoft commented Nov 26, 2020

This blurs the line a little if someone is using morphs. For example, what is our recommendation on when to use morphs vs cable_ready directly?

@julianrubisch
Copy link
Contributor

Can we use this also to decouple the broadcasters?

Comes to mind because I mocked CableReady::Channels pretty often in the tests

@leastbad
Copy link
Contributor

leastbad commented Nov 26, 2020

Someone wise suggested that the easy way to decide if you need a StimulusReflex#morph vs a CableReady operation is whether you need StimulusReflex callbacks on the client afterwards.

@hopsoft
Copy link
Contributor Author

hopsoft commented Nov 26, 2020

@julianrubisch Yes, I expect this to open up some refactoring opportunities in the broadcasters, tests, and possibly elsewhere.

@julianrubisch
Copy link
Contributor

So I‘m playing bad cop (again) - could you add some exemplary tests for StimulusReflex::CableReadyChannels ? This will further (my) understanding and facilitate future refactorings...

@hopsoft
Copy link
Contributor Author

hopsoft commented Nov 26, 2020

Yes and thank you for pushing hard on the tests @julianrubisch.

@RolandStuder
Copy link
Contributor

RolandStuder commented Nov 26, 2020

Nice stuff!

We should be careful not to encourage element mutations via CableReady if/when someone is using default reflex behavior.

Could the proxy object not actually somehow prevent that by declaring a morph :nothing? <- stupid idea, of course you would want to be able to have selector morphs and such cool new cable_ready calls in on reflex

@hopsoft hopsoft marked this pull request as draft November 26, 2020 20:26
@leastbad
Copy link
Contributor

Another excellent reason to require broadcast is that - at least right now - if they piggyback CR operations that aren't SR-specific on a broadcast that has SR operations, well, very bad things. As currently implemented, it would actually break because the code assumes every dispatchEvent/morph/innerHtml has a stimulusReflex accessor. All other operations would be silently discarded. Obviously not desireable.

I am going to have to make another pass on received that is more robust. I need to separate the data.operations into two objects: one that has stimulusReflex accessors and one that doesn't.

@hopsoft hopsoft marked this pull request as ready for review November 27, 2020 21:50
@hopsoft hopsoft merged commit 57f4553 into master Nov 28, 2020
@hopsoft hopsoft deleted the hopsoft/cr-proxy branch November 28, 2020 14:16
julianrubisch added a commit to julianrubisch/stimulus_reflex that referenced this pull request Dec 25, 2020
* Exit with nonzero status code

* [nodoc] update changelog

* GitBook: [master] 3 pages modified

* [nodoc] update changelog

* Handles to mitigate race conditions when running reflexes in quick succession on the same element (stimulusreflex#377)

* element.reflexData becomes a dictionary

Co-authored-by: leastbad <[email protected]>

* [nodoc] update changelog

* fixed bug preventing callbacks for multiple morphs

* [nodoc] update changelog

* GitBook: [master] 5 pages modified

* GitBook: [master] one page modified

* GitBook: [master] one page modified

* [nodoc] update changelog

* GitBook: [master] one page modified

* [nodoc] update changelog

* make element.reflexController a dictionary (stimulusreflex#379)

make element.reflexController a dictionary

* [nodoc] update changelog

* Move package.json to root of project (stimulusreflex#380)

* Move package.json to root of project
* Update readme instructions for publish

* [nodoc] update changelog

* Prep for v3.4.0.pre5 release

* Update to use gitcdn for logo on readme

* Bump js package version

* Return from received if isolationMode: true

* [nodoc] update changelog

* GitBook: [master] one page modified

* [nodoc] update changelog

* non-SR cable_ready operation pass-through (stimulusreflex#381)

* works like a charm

* a superior solution

* [nodoc] update changelog

* Set reflexController to empty object

* 800

* [nodoc] update changelog

* GitBook: [master] one page modified

* [nodoc] update changelog

* don't show findElement warnings unless debugging (stimulusreflex#384)

* dont exit in sanity checker on stimulus_reflex:install

* split SR operations from data.operations (stimulusreflex#385)

* [nodoc] update changelog

* [nodoc] update changelog

* add jQuery support to SR events (stimulusreflex#388)

* Update templates for new stage etc (stimulusreflex#390)

* updated templates for new lifecycle stages

* added accessors to Reflex class

* [nodoc] update changelog

* reflexError and received refactor (stimulusreflex#389)

* reflexError and received refactor

* strict equality checks + simpler existential checks

* Setup a proxy object that wraps CableReady::Channels (stimulusreflex#382)

Setup a proxy object that wraps CableReady::Channels

This improves developer ergonimics for emitting CR broadcasts on the SR
channel within a reflex.

* Remove codeclimate

* [nodoc] update changelog

* Add codacy config file

* [nodoc] update changelog

* Have codacy ignore markdown files

* Allow `success` and `after` lifecycle methods on replaced elements (stimulusreflex#386)

* Allow `success` and `after` lifecycle methods on replaced elements

* use destruction syntax

* Extract current debug setting into debug.js to sync the value between files

* Log warnings if users morphs the element which triggered the reflex.

* Log extra warning if the element is not present anymore to state, that the lifecycle methods were not executed

* standardize

* Tweaked wording slightly

* added reflexError handling to dispatchLifecycleEvent

Co-authored-by: leastbad <[email protected]>
Co-authored-by: leastbad <[email protected]>

* [nodoc] update changelog

* Prep for 3.4.0.pre6 release

* [nodoc] update changelog

* Update js package version

* [nodoc] update changelog

* Update %file_name%_reflex.rb.tt

* Update stimulus_reflex.js

* [nodoc] update changelog

* Update lifecycle.js

* tweaked life-cycle messages

* Add check to return early if element is undefined in invokeLifecycleMethod

* don't warn folks twice (stimulusreflex#392)

* [nodoc] update changelog

* [nodoc] update changelog

* Update stimulus_reflex.rb

* [nodoc] update changelog

* GitBook: [master] 12 pages and 11 assets modified

* Trigger piggy back operations after SR operations (stimulusreflex#395)

* Trigger piggy back operations after SR operations
* Refactor proxy object to be explicit about what is delegated.
* Update to latest CableReady

* [nodoc] update changelog

* Prep for 3.4.0.pre7

* [nodoc] update js package version to 3.4.0-pre7

* GitBook: [master] 3 pages modified

* [nodoc] update changelog

* GitBook: [master] one page modified

* [nodoc] update changelog

* GitBook: [master] one page modified

* [nodoc] update changelog

* Delegate render to controller (stimulusreflex#397)

* Delegate render to controller

* Stand up basic testing for reflex rendering

* Delegate render to the controller class rather than instance

* Also, refactor some internals to match Rails internals.

* [nodoc] update changelog

* [nodoc] prepare for pre8

* [nodoc] update js package to pre8

* GitBook: [master] 14 pages modified

* [nodoc] update changelog

* GitBook: [master] one page modified

* [nodoc] update changelog

* GitBook: [master] 14 pages modified

* [nodoc] update changelog

* GitBook: [master] 14 pages modified

* [nodoc] update changelog

* loosen stimulus dev dependency

* GitBook: [master] 4 pages modified

* [nodoc] update changelog

* GitBook: [master] 11 pages modified

* [nodoc] update changelog

* GitBook: [master] 2 pages modified

* [nodoc] update changelog

* GitBook: [master] 14 pages modified

* GitBook: [master] 2 pages modified

* GitBook: [master] one page modified

* an install that actually works on a fresh Rails new

* GitBook: [master] one page modified

* [nodoc] update changelog

* Don't memoize the singleton (stimulusreflex#400)

* [nodoc] update changelog

* GitBook: [master] 2 pages modified

* [nodoc] update changelog

* Add dom_id to the reflex (stimulusreflex#405)

* [nodoc] update changelog

* Check package version from yarn.lock if node_modules folder is not available. Fixex stimulusreflex#402 (stimulusreflex#403)

* [nodoc] update changelog

* Don't run sanity checker in production (stimulusreflex#404)

* Allow StimulusReflex to process Rack middlewares (stimulusreflex#399)

* Allow StimulusReflex to process Rack middleswares

* Possibly fixes stimulusreflex#289

* process the whole Rails middleware stack

* just call the Rails middleware stack if it reponds to `build`

* pass routes to middleware build method

* make middleware processing configurable

* use ActionDispatch::MiddlewareStack to register middlewares

* update comment in initializer

* add note so people know what to do if they encounter the 'No route matches' error

* Update channel.rb

* Update stimulus_reflex.rb

Co-authored-by: leastbad <[email protected]>

* [nodoc] prep for v3.4.0.pre9

* [nodoc] update js package version

* GitBook: [master] one page modified

* [nodoc] update changelog

* GitBook: [master] 2 pages modified

* [nodoc] update changelog

* GitBook: [master] one page modified

* [nodoc] update changelog

* GitBook: [master] one page modified

* GitBook: [master] one page modified

* [nodoc] update changelog

* GitBook: [master] one page modified

* [nodoc] update changelog

* GitBook: [master] one page modified

* [nodoc] update changelog

* GitBook: [master] one page modified

* [nodoc] update changelog

* GitBook: [master] one page modified

* [nodoc] update changelog

* Update README.md

* [nodoc] update changelog

* GitBook: [master] one page modified

* [nodoc] update changelog

* GitBook: [master] 2 pages modified

* [nodoc] update changelog

* GitBook: [master] one page modified

* [nodoc] update changelog

* GitBook: [master] one page modified

* [nodoc] update changelog

* [nodoc] prep for v3.4.0

* [nodoc] update npm package version

* GitBook: [master] one page modified

* [nodoc] update changelog

* GitBook: [master] one page modified

* [nodoc] update changelog

* GitBook: [master] one page modified

* [nodoc] update changelog

* GitBook: [master] one page modified

* GitBook: [master] one page modified

* [nodoc] update changelog

* fix 'operartion' typo

* [nodoc] update changelog

* Update README.md

Co-authored-by: leastbad <[email protected]>
Co-authored-by: GitHub Actions <[email protected]>
Co-authored-by: leastbad <[email protected]>
Co-authored-by: Rafe Rosen <[email protected]>
Co-authored-by: Nate Hopkins <[email protected]>
Co-authored-by: Marco Roth <[email protected]>
Co-authored-by: Roland Studer <[email protected]>
Co-authored-by: Josh LeBlanc <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants