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

Testing out disabling Reagent ratom/rendering queue #612

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Deraen
Copy link
Member

@Deraen Deraen commented Nov 15, 2024

#504 etc.

Currently test suite is getting stuck due to test using u/with-mounted-component-async and u/run-fns-after-render not finishing at all.

TODO:

  • Figure out how to call after-render callbacks after React finishes the render
  • New render and act utils to run checks after component is rendered
  • Check and consider if we can just completely drop create-class and the class-based component implementation
    • Probably not a required change, but would make reagent code cleaner
  • drop reagent.core/after-render and possibly other queue functions
    • There is no simple way to hook into React rendering queue to keep this working as previously.
      Likely this isn't used that often, so users would just replace these places with hooks manually?

(is (= 2 @ran) "ran once more")
;; NOTE: OKAY Here is a problem:
;; reactions are now being run for each input ratom change because we don't have the queue anymore!
(is (= 2 @reaction-ran))
Copy link
Member Author

Choose a reason for hiding this comment

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

As I was thinking, while components aren't rendered for each Ratom change due to the React queue, here the reaction body is running each time input Ratoms change, which is potentially a (big) problem for re-frame.

Consider cases where text input value is used as a reaction (or subscription) input.

@Deraen Deraen force-pushed the reagent-next branch 5 times, most recently from 46a63a1 to 9cc6842 Compare November 28, 2024 11:33
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.

1 participant