Skip to content
New issue

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

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

Already on GitHub? # to your account

[Enhancement]: Allow subscribe to be safely called from anywhere #753

Open
dannyfreeman opened this issue Mar 6, 2022 · 0 comments
Open
Assignees

Comments

@dannyfreeman
Copy link
Contributor

dannyfreeman commented Mar 6, 2022

What do you suggest?

This ticket is a followup from PR #752 (comment)

Enhancement Request

I would like to be able to call subscribe anywhere in my re-frame app without the risk of leaking memory in the subscription cache. Specifically, I would like to be able to call subscriptions from inside events. There is a lot of useful business logic performed in subscriptions that can be re-used in event handlers and other non-reactive contexts.

Current workarounds

If the event is close to the view (I.E. dispatched directly in a click handler or something like that), I use the subscription inside view and pass the value to the event when dispatching. This is usually really easy when I also need the subscription value in the view itself

Sometimes my event is a little further down the dispatch chain, or dispatched by many views that don't need to subscribe to the same data themselves. In this situation I extract subscription handlers into standalone functions with a signature of db & args and call that function from the subscription handler and the event. This works fine for simple layer-2 subscriptions. For layer 3 subscriptions I have to re-create the input signal graph manually. This means extracting regular functions out of many subscription handlers and composing the results, a process that can be messy and prone to error.

Proposed Solution

When calling subscribe and a reaction is not present in the subscription cache, check that the current procedure is being called in a reactive process before caching. If it is not reactive, bypass caching the reaction. I have a patch in the works for this here: #754

This could result in a lot of cache misses when calling subscriptions from events, but the performance penalty is nearly identical to the workaround I most commonly employ, which is extracting subscription handlers into functions and invoking those functions every time the event handler executes. If the subscription is cached then the event handler will be able to use the cached reaction, which is a great bonus.

Other possible solutions

Some other solutions I have entertained: a special subscribe function that is only intended to be used outside reagent components, or a co-effect for injecting subscription values in events.
Having different semantics for different contexts could have it's own advantages, like keeping the existing "don't sub outside a component" warnings where people really shouldn't be subscribing, like an on-click callback. The proposed solution above wouldn't be able to differentiate between that situation and subscribing in an event (maybe it could with some dynamic binding magic)

dannyfreeman added a commit to dannyfreeman/re-frame that referenced this issue Mar 6, 2022
The `warn-when-not-reactive` warnings are no longer valid if we bypass
the subscription cache when not in a reagent component, so they've been
removed.

This should address issue day8#753

Still needs some testing, and probably some documentation cleanup
dannyfreeman added a commit to dannyfreeman/re-frame that referenced this issue Mar 6, 2022
The `warn-when-not-reactive` warnings are no longer valid if we bypass
the subscription cache when not in a reagent component, so they've been
removed.

This should address issue day8#753

Still needs some testing, and probably some documentation cleanup
dannyfreeman added a commit to dannyfreeman/re-frame that referenced this issue Mar 6, 2022
The `warn-when-not-reactive` warnings are no longer valid if we bypass
the subscription cache when not in a reagent component, so they've been
removed.

This should address issue day8#753

This change probably requires some documentation cleanup
dannyfreeman added a commit to dannyfreeman/re-frame that referenced this issue May 13, 2022
The `warn-when-not-reactive` warnings are no longer valid if we bypass
the subscription cache when not in a reagent component, so they've been
removed.

This should address issue day8#753

This change probably requires some documentation cleanup
@kimo-k kimo-k self-assigned this Jul 30, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants