preactjs / signals

Manage state with style in every framework
https://preactjs.com/blog/introducing-signals/
MIT License
3.82k stars 95 forks source link

Subscription integration for third-party libraries #351

Open nahudalla opened 1 year ago

nahudalla commented 1 year ago

Hello, I'm experimenting with Preact and Signals on a new side project I started recently. I really like the simplicity and speed so far.

TL;DR: I'd like to ask if it's possible to change something, perhaps add a new "hooks" functionality to "hook into" the subscription mechanism of the Signal class.

For this new side project, I'm experimenting with new ways to separate as much as possible my app's logic and data fetching from the UI layer. I think Signals are a great way of accomplishing just that, however I'd also like non-UI code to remain framework- and library-agnostic.

Because of that, I'm trying to use Preact's Signals in my UI layer but I'm experimenting with the standard EventTarget API for my non-UI code. This brings me to the point of this issue, which is that there's currently no way to detect when a new subscriber is added or removed from a Signal. I need this functionality to be able to add and remove subscriptions to external event sources only when the Signal is actually being used, and thus avoiding dangling subscriptions.

Currently, I'm trying to expand the default Signal functionality by extending the class. By looking at this package's source code I've noticed that there are two internal methods that I'd like to override (_subscribe and _unsubscribe) to detect when a subscription is added and removed. However, since these methods are marked as @internal, the minified version of the library has different (shorter) names for these specific methods and therefore cannot be overridden by using their original name.

I'd like to ask if it's possible to change something, perhaps add a new "hooks" functionality to "hook into" the subscription mechanism of the Signal class. I'd be more than happy to help with the implementation of this new feature and create a PR but since I haven't worked with this code before, maybe I'm missing something and what I want shoulnd't be done for some reason.

PD: Sorry for the wall of text!

rschristian commented 1 year ago

However, since these methods are marked as @internal, the minified version of the library has different (shorter) names for these specific methods and therefore cannot be overridden by using their original name.

We use a mangle.json file to ensure consistent mangling. They're always converted to S and U respectively. Patch them as you please.

nahudalla commented 1 year ago

We use a mangle.json file to ensure consistent mangling. They're always converted to S and U respectively. Patch them as you please.

Thank you! This solves my problem in the short term. However, wouldn't it still be great to have an official, documented way to hook into the subscription mechanism? I think _subscribe and _unsubscribe are implementation details and could be changed/removed at any time without warning.

The way I'm implementing this, since I'm using the standard EventTarget API, allows signals to be easily used for other things such as tracking the pointer (mouse) position, tracking network online state, or anything else the browser provides events to notify of the changes. I could publish this as a utility library to be used with Preact's Signals, but I'd like a more robust integration approach with guarantees that it won't change between minor versions.

rschristian commented 1 year ago

However, wouldn't it still be great to have an official, documented way to hook into the subscription mechanism? I think _subscribe and _unsubscribe are implementation details and could be changed/removed at any time without warning.

I can't say I'm a fan of that myself, but indeed, it could potentially change in a major. There are no plans to do this, however.

pjeby commented 1 year ago

One potential way an integration API could work would be to change the signal() function signature thus:

function signal<T, S = () => void>(
    value: T,
    subscribe?: (signal: Signal<T>) => S,      // set up upstream source to update .value
    unsubscribe?: (subscription: S) => void    // turn off upstream source
): Signal<T> {
    // ...
}

The idea is that if given, subscribe(signal) is called when the signal enters an in-use state, and the return value is saved. When the signal is no longer in use, call unsubscribe() with the saved value, or call the saved value if no unsubscribe function was given. This protocol is sufficient for all sorts of possible upstreams, including DOM events, node events, rxjs observables, Obsidian eventrefs, etc., but would still allow the library's internals to be kept hidden.

mbeckem commented 3 months ago

This looks like a duplicate of #428. Given the interest in this feature, would a PR for this have a chance of getting merged?

I would like to propose the following API, which does not add an additional type parameter to the signal function:

function signal<T>(value: T, options?: {
  // Called when the signal starts being used by an effect / computed signal, when it was previously unused.
  watched?: (signal: Signal<T>): void;
  // Called when the signal is no longer used by any effects / computed signals, when it was previously being used.
  unwatched?: (signal: Signal<T>): void;

  // or subscribed / unsubscribed
}): Signal<T>

Note that computed() should also support these callbacks. As an alternative, these functions could also be implemented on the Signal class, as public methods.