Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Signal.subtle.watched/unwatched is not sufficient, we need to be notified when a signal is used (even without a watcher) #227

Open
divdavem opened this issue Jun 12, 2024 · 5 comments

Comments

@divdavem
Copy link

divdavem commented Jun 12, 2024

Hello,
As a maintainer of the tansu signal library, I am trying (here) to re-implement it using signal-polyfill and I came across the following difference of behavior, that I would like to solve, avoiding any breaking change.

With tansu (inspired by svelte stores), as described further in this article we have the ability to know whether a signal is used or not, in order to update the signal value and add or remove event listeners.
For example, we can create the following signal that contains a string value from the local storage and a computed that parses the value as JSON:

import {readable, computed} from '@amadeus-it-group/tansu';

// this signal is synchronized with the "preferences" item in local storage
const preferences$ = readable(null as string | null, (set) => {
  // this function is called when the signal starts to be used

  const updateFromStorage = (e: StorageEvent) => {
    if (e.key === "preferences") {
      set(e.newValue);
    }
  };
  window.addEventListener('storage', updateFromStorage);
  set(localStorage.getItem("preferences"));
  return () => {
    // this function is called when the signal is no longer used
    window.removeEventListener('storage', updateFromStorage)
  };
});

const parsedPreferences$ = computed(() => JSON.parse(preferences$() ?? "null"));

Now if I call parsedPreferences$() at any time, I have the current value of the "preferences" local storage item parsed as a json object.

However, with the current signals proposal, this is not possible to achieve without constantly listening to the "storage" event.

I know the Signal.subtle.watched and Signal.subtle.unwatched callbacks, but they are not called when the value is read through a one-time access to the value of a dependent signal:

import { Signal } from 'signal-polyfill';

const updateFromStorage = (e: StorageEvent) => {
  if (e.key === "preferences") {
    preferences$.set(e.newValue);
  }
}
const preferences$ = new Signal.State(null as string | null, {
  [Signal.subtle.watched]: () => {
    preferences$.set(localStorage.getItem("preferences"));
    window.addEventListener('storage', updateFromStorage);
  },
  [Signal.subtle.unwatched]: () => {
    window.removeEventListener('storage', updateFromStorage);
  }
})

const parsedPreferences$ = new Signal.Computed(() => JSON.parse(preferences$.get() ?? "null"));

With the above code, if I read parsedPreferences$.get() once (without having a watcher) and then the value of the "preferences" local storage item changes from another tab, and then I read again (still without having a watcher) parsedPreferences$.get(), the value will not be up-to-date.

I believe it is fundamental that the signals proposal allows such a use case.

@PsychoLlama
Copy link

I ran into this problem in my framework too. It seems this proposal allows an unavoidable edge case when integrating external sources where the replica might be stale and there's no reliable way to know.

I've opened a broader proposal in #237.

@shaylew
Copy link
Collaborator

shaylew commented Aug 24, 2024

One tricky part here is if you want a notion of "is this signal in use?" that incorporates unwatched signals, whatever that notion is ends up sensitive to GC timing: a computed that previously read your signal can become garbage later, without ever having another lifecycle event happen to it.

If you accept that condition, you can deal with it explicitly and get something resembling... well, it resembles what you asked for, though not necessarily what you want. What you'd do is:

  • keep an "internal" State that you update (via an event listener)
  • when someone wants access to it, you hand out a "public" Computed that reads the private state. you put the public computed in a FinalizationRegistry, and you make sure that (if you retain it at all) you only retain the public Computed weakly
  • now you can use "are there any public readers that haven't been finalized yet?" to tell whether there's any possibility of any other code trying to consume the value, which would let you add or clean up the handlers.

Of course, it may be that what you really want is more like #237 (a way to make things recompute every time no matter what) rather than a way to be notified when all (watched and unwatched both) consumers of a node become garbage.

@divdavem
Copy link
Author

divdavem commented Aug 27, 2024

@shaylew Thank you for your comment.

One tricky part here is if you want a notion of "is this signal in use?" that incorporates unwatched signals, whatever that notion is ends up sensitive to GC timing: a computed that previously read your signal can become garbage later, without ever having another lifecycle event happen to it.

I don't see things like this. For me, GC should not be involved at all in the process of knowing whether a signal is in use.
A signal can be and should be able to be considered unused even though it is not garbage collected. Maybe it will be in use later.

For me, a signal should be considered in use:

  • if it is watched
  • or synchronously from just before being read to just after

That's what we implemented in tansu: there is only one way to read the value of a store which is subscribing to it. As long as there is a subscription, the store is used. When the last subscriber unsubscribes, the store is unused.
Then we have a helper get function that allows to get the value of the store. It just internally subscribes, which gives the value of the store to the subscriber synchronously, and then immediately unsubscribes.

Of course, it may be that what you really want is more like #237 (a way to make things recompute every time no matter what) rather than a way to be notified when all (watched and unwatched both) consumers of a node become garbage.

What I really want is indeed similar to #237.

@shaylew
Copy link
Collaborator

shaylew commented Aug 27, 2024

That's what we implemented in tansu: there is only one way to read the value of a store which is subscribing to it. As long as there is a subscription, the store is used. When the last subscriber unsubscribes, the store is unused.

Ah, I see how you can make things work with that sort of design. But one of the goals of signals is for APIs that are resting to be able to be used from code that doesn't know or care about signals. If you can call .get from anywhere, and consumers don't need to manually subscribe and unsubscribe, "is it still live" is the only real approximation you can get of whether it might be accessed. As long as it's possible to read a signal from a regular non-signal context, you're going to have readers that the framework didn't know about ahead of time.

(And "use watchers to keep things live" basically corresponds to the other answer, which is "okay, I'll trade manual subscribe/unsubscribe for more precise tracking of what's 'active'".)

@divdavem
Copy link
Author

For info, I have opened this PR in the polyfill as a potential solution for this issue.

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

No branches or pull requests

3 participants