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

New CR wire format + reworked server message events #536

Merged
merged 8 commits into from
Aug 22, 2021

Conversation

leastbad
Copy link
Contributor

@leastbad leastbad commented Aug 5, 2021

Type of PR (feature, enhancement, bug fix, etc.)

Enhancement

Designed to work with stimulusreflex/cable_ready#142 - you will need to check out both branches and use local bundles and yarn link (or whatever you do when you do this) to make everything connect up.

Description

  1. Reflex code adapted to handle proposed new CableReady wire format. The data structure is as simple as can be, making it much easier to work with operations in SR. This presented an opportunity to refactor several parts of the codebase which I believe were overly complex and discouraged hacking by people who aren't me.
  2. This PR also proposes a refactoring of what used to be the concept of serverMessages; an overloaded concept that includes: Nothing Reflexes, Reflexes with server errors and halted Reflexes. These are now all individual methods. The client callbacks have been split out in a very readable way. Again, I anticipate that this will make it much easier for other people to engage.

One interesting aspect of this PR is that I had to add before-dispatch-event and specifically, after-dispatch-event callbacks to CableReady's dispatch_event operation. This is all in service of removing the repetitive event.detail ? event.detail.stimulusReflex : event.stimulusReflex logic that litered the library previously. That's because when we are working with CableReady operations, we extract data from the CableReady operation as well as the event itself. In fact, the stimulus-reflex:morph-nothing (and morph-halted and morph-error) events are largely unused, except to obtain the type of the currently running Reflex.

Why should this be added

If we adopt the new CR wire format, much of this would be necessary regardless.

That said, these changes allowed me to remove rough edges and ugly, confusing bits from several important mechanisms. I believe that the library is now significantly easier to refactor, test and add new features to.

Checklist

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

@leastbad leastbad added enhancement New feature or request proposal ruby Pull requests that update Ruby code javascript Pull requests that update Javascript code cableready labels Aug 5, 2021
@leastbad leastbad added this to the 3.5 milestone Aug 5, 2021
@leastbad leastbad self-assigned this Aug 5, 2021
@leastbad leastbad merged commit 2f55c7e into stimulusreflex:master Aug 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cableready enhancement New feature or request javascript Pull requests that update Javascript code proposal ruby Pull requests that update Ruby code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants