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

Fix timing issues with post-Reflex lifecycle callbacks #299

Merged
merged 3 commits into from Sep 13, 2020

Conversation

leastbad
Copy link
Contributor

@leastbad leastbad commented Sep 7, 2020

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

Bug fix

Description

This PR tackles the issue of afterReflex callbacks firing before CableReady operations can be completed, as well as some other housekeeping concerns.

When we added data-reflex-root support to SR, we introduced the last flag to the CableReady payload. This was always brittle and should have been removed when we added reflexId as a concept. Instead, we had many users reporting intermittent issues with their afterReflex callbacks either not firing, or firing but not seeing visual evidence. This is because each morphdom process takes several ms. The fix introduced in #286 was unfortunately ineffective as a remedy.

I introduced the concept of attaching totalOperations and completedOperations to the promise tracking the Reflex, and removed the last flag entirely as it is no longer required. Each time CableReady finishes an operation, the completedOperations count is increased until all have been accounted for, and the callbacks are executed.

I also made sure that the callback events, promises and functions all receive reflexId. You can access the reflexId of an unresolved promise by accessing promise.reflexId, which is pretty fun. Developers can now confidently track the status of reflex operations as uniquely identified transactions. The reflexId is the new last argument of every function. Developers might need to specify an unused error argument placeholder in their event handlers to access the reflexId; this is not a breaking change because arity was not changed.

As part of this update, I modified the createSubscription function to be aware of both morph and innerHtml functions, as per changes in the SelectorBroadcaster class.

Finally, I removed the unused morphMode key from the promise response.

Fixes #281

Why should this be added

This should unblock v3.3 for final review and testing.

Checklist

  • My code follows the style guidelines of this project
  • Checks (StandardRB & Prettier-Standard) are passing

@leastbad leastbad self-assigned this Sep 7, 2020
@leastbad leastbad added the bug Something isn't working label Sep 7, 2020
@leastbad leastbad added this to the 3.3.0 milestone Sep 7, 2020
@leastbad leastbad merged commit 5e2450b into stimulusreflex:master Sep 13, 2020
@leastbad leastbad deleted the after_reflex branch September 13, 2020 03:30
jonathan-s added a commit to jonathan-s/django-sockpuppet that referenced this pull request Oct 17, 2020
jonathan-s added a commit to jonathan-s/django-sockpuppet that referenced this pull request Oct 17, 2020
jonathan-s added a commit to jonathan-s/django-sockpuppet that referenced this pull request Oct 18, 2020
jonathan-s added a commit to jonathan-s/django-sockpuppet that referenced this pull request Oct 18, 2020
shipinstorm added a commit to shipinstorm/django-sockpuppet that referenced this pull request Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lifecycle callbacks do not work
1 participant