Skip to content

Fix unsubscribe not called when: disposing an observe on a child node or unmounting a synced useObservable#622

Open
msantang78 wants to merge 2 commits intoLegendApp:mainfrom
msantang78:fix/child-observe-synced
Open

Fix unsubscribe not called when: disposing an observe on a child node or unmounting a synced useObservable#622
msantang78 wants to merge 2 commits intoLegendApp:mainfrom
msantang78:fix/child-observe-synced

Conversation

@msantang78
Copy link
Contributor

@msantang78 msantang78 commented Dec 16, 2025

This PR is a proposed fix for #621 and #624

It solves:

1. Unsubscribe is not called when disposing an observe on a child node #621

const store = observable(
  synced({
    initial: { value: 1 },
    subscribe: ({ update }) => {
      console.log("subscribed");
      return () => {
        console.log("unsubscribed");
      };
    },
  })
);

const u = observe(() => {
  store.value.get();
});

u();

Changes

  • It ensures that listeners-cleared is triggered for the parents node when there is no more listeners
  • It also triggers the listener-added for parents (it was not triggered again after the first un-subscription)

At the moment, listener-added is only triggered for synced nodes due to the test “should only trigger middleware for the registered node”. It’s unclear whether this is the intended behavior or if I should also trigger for all parent nodes.

2. Unsubscribe not called when unmounting a synced useObservable #624

const obs$ = useObservable(
    synced({
        initial: 0,
        subscribe: () => {
            numSubscribes++;
            return () => {
                numUnsubscribes++;
                console.log('unsubscribed'); // <--- never executed
            };
        },
    }),
);

Changes

  • Added tests for useObservable
  • Ensure listeners-cleared is dispatched when unmounted

@msantang78
Copy link
Contributor Author

msantang78 commented Dec 26, 2025

I also found that:

  • The synced observables are not unsubscribed on unmount
  • The synced observables are not reset when the useObservable dependencies changes

Besides the missing functionality this should be causing memory leaks.
I'm will try to add some extra changes to this PR to fix those cases too

@msantang78 msantang78 changed the title Fix: Unsubscribe is called for synced observable only on root node Fix unsubscribe not called when: disposing an observe on a child node or unmounting a synced useObservable Dec 26, 2025
@msantang78 msantang78 force-pushed the fix/child-observe-synced branch from c33e5e8 to e26d500 Compare December 26, 2025 22:20
@msantang78 msantang78 force-pushed the fix/child-observe-synced branch 2 times, most recently from ac6dc7a to dd3e67a Compare December 28, 2025 17:49
@msantang78
Copy link
Contributor Author

@jmeistrich sorry to tag you, but this one seems important, It leads to memory leaks and not been able to use subscriptions. Can you please check it out when you have a moment 🙏

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