fix(signals): use Injector of rxMethod instance caller if available
#4529
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for ngrx-io canceled.
|
rainerhahnekamp
force-pushed
the
fix/signals/rx-method-unclosed-effects
branch
from
c207a69
to
7286585
Compare
Ensure that an instance of `rxMethod` uses the `Injector` of the caller where the instance was invoked, rather than the injector of the node where `rxMethod` was created. This change addresses a potential memory leak where a `SignalStore` method, generated by `rxMethod` and provided at the root level, continues to track a signal via its `effect` using the `RootInjector`. In cases where the component calling the method is destroyed, the `Signal` remains tracked, leading to unintended behavior and memory retention. With this commit, `rxMethod` now prioritizes the injector of the instance caller, falling back to the creator's injector only if no injection context exists. Additionally, a custom injector can be provided by the caller and will take precedence when specified. For `Observable`, since it has a different completion mechanism, the instance caller is still responsible for handling unsubscription or completion.
rainerhahnekamp
force-pushed
the
fix/signals/rx-method-unclosed-effects
branch
from
7286585
to
720ec77
Compare
There was a problem hiding this comment.
Left a few suggestions. It will be useful to add a concise section on this topic to the RxJS Integration page.
Co-authored-by: Marko Stanimirović <[email protected]>
…ub.com/rainerhahnekamp/ngrx into fix/signals/rx-method-unclosed-effects
|
@markostanimirovic, I've incorporated all your suggestions and fixed the issues with the source subscription. As a result, the current version of If you are OK with the implementation, I'd do a last push and add the addition for the docs. |
Co-authored-by: Tim Deschryver <[email protected]>
2 tasks
markostanimirovic
changed the title
Injector of rxMethod instance callerInjector of rxMethod instance caller if available
markostanimirovic
force-pushed
the
fix/signals/rx-method-unclosed-effects
branch
2 times, most recently
from
9c9913a
to
bb79531
Compare
markostanimirovic
force-pushed
the
fix/signals/rx-method-unclosed-effects
branch
from
bb79531
to
b3fcf82
Compare
markostanimirovic
approved these changes
Sep 28, 2024
timdeschryver
approved these changes
Sep 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ensure that an instance of
rxMethoduses theInjectorof the caller where the instance was invoked, rather than the injector of the node whererxMethodwas created.This change addresses a potential memory leak where a
SignalStoremethod, generated byrxMethodand provided at the root level, continues to track a signal via itseffectusing theRootInjector. In cases where the component calling the method is destroyed, theSignalremains tracked, leading to unintended behavior and memory retention.With this commit,
rxMethodnow prioritizes the injector of the instance caller, falling back to the creator's injector only if no injection context exists. Additionally, a custom injector can be provided by the caller and will take precedence when specified.For
Observable, since it has a different completion mechanism, the instance caller is still responsible for handling unsubscription or completion.PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Closes #4528
What is the new behavior?
Does this PR introduce a breaking change?