Enhance Angular reactivity adapter

[makuko]

I created my own Angular reactivity adapter to support scope checking and automatic cleanup. I think it would be nice if this was integrated into the official adapter.

At least scope checking should be included since Angular provides the neccessary method.

The thin wrapper around Angular's effect also seems to work flawlessly in my tests. @maxnowack It would be great to hear your opinion about potential issues with this approach.

Here's the code:

import { assertNotInReactiveContext, CreateEffectOptions, effect, EffectCleanupRegisterFn, isDevMode, signal, untracked } from '@angular/core';
import { createReactivityAdapter } from '@signaldb/core';

function isInScope() {
    try {
        assertNotInReactiveContext(isInScope);

        return false;
    } catch {
        return true;
    }
}

let onDispose: EffectCleanupRegisterFn | undefined;

export function dbEffect(effectFn: (onCleanup: EffectCleanupRegisterFn) => void, options?: CreateEffectOptions) {
    return effect(
        onCleanup => {
            onDispose = onCleanup;

            effectFn(onCleanup);

            onDispose = undefined;
        },
        options
    );
}

const reactivityAdapter = createReactivityAdapter({
    create: () => {
        const dep = signal(0);

        return {
            depend: () => {
                dep();
            },
            notify: () => dep.set(untracked(dep) + 1)
        };
    },
    isInScope,
    onDispose: fn => {
        if (isDevMode() && !onDispose) {
            console.warn("SignalDB was used in a reactive context that doesn't support automatic cleanup. Consider using `dbEffect`.");
        }

        onDispose?.(fn);
    }
});

export default reactivityAdapter;

[maxnowack]

Hi @makuko! Thanks for sharing! 🙂
I didn't know the assertNotInReactiveContext method. That`s a good solution to check whether a reactive scope is active.

Regarding the effect function: That`s a neat trick to solve the cleanup issue. I currently see two issues with this implementation:

1. You have to make sure that every part where the effect function and SignalDB queries are used, uses the patched implementation for it to work. There might be places in the framework where you can't patch the effect function because the original one is called internally. I'm not sure how much of a real problem this is.

2. Your current implementation only supports one cleanup function. While this is fine for typical usage, it won't work with nested effect calls. I'm not sure if nested effects are a common use case in Angular, but if needed, they could be detected and a warning could be shown to the user.

Would you mind opening a PR for the scope checking? For the patched effect method, it would be great to hear some feedback from other users as well. This approach could also be implemented for other frameworks where no automatic cleanup is available 🙂

[makuko]

Hey @maxnowack, thanks for your quick feedback! Regarding your comments:

1. I think this should not be an issue because as far as I can see having the patched effect but not using it won't break anything. In fact the console.warn I added will actually give developers a reminder that cleanup is necessary. Either by changing to the patched effect or by doing manual cleanup which would be needed anyway.
2. I don't know about other frameworks but at least in the case of Angular nested effects are not allowed and will throw an error. Angular actually uses the assertNotInReactiveContext for that internally. So this would not be a problem and if more cleanup is needed the core cleanup function can register multiple functions therefore I hand it over to effectFn.