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

Rewriting graph implementation, a signal is made live while getting its value #32

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

divdavem
Copy link
Contributor

@divdavem divdavem commented Sep 12, 2024

This PR contains a full rewrite of the graph implementation with a bit less code.

I am opening this PR as a possible solution for the following issue I opened: tc39/proposal-signals#227
With this PR, reading a signal outside a reactive context makes it live for the duration of the get. So, calling get on a signal that is not live triggers the watched callback and then the unwatched callback of the signal itself (and all depending signals that were not live).

With this PR, it is possible to do this:

import { Signal } from 'signal-polyfill';

const updateFromStorage = (e: StorageEvent) => {
  if (e.storageArea === localStorage && 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"));

And parsedPreferences$() will always contain the up-to-date value.

This PR also includes the tests and corresponding behavior from #15 and #16.
I have disabled 2 tests that I think are invalid.

@divdavem divdavem changed the title Reading a computed makes it live for the duration of get Reading a signal makes it live for the duration of get Sep 13, 2024
@divdavem divdavem changed the title Reading a signal makes it live for the duration of get [WIP] Reading a signal makes it live for the duration of get Sep 13, 2024
@divdavem divdavem force-pushed the getMakesLive branch 5 times, most recently from 9298ee4 to b6f5d4f Compare September 18, 2024 13:13
@divdavem divdavem changed the title [WIP] Reading a signal makes it live for the duration of get Rewriting graph implementation, a signal is made live while getting its value Sep 18, 2024
@littledan
Copy link
Contributor

Thank you for this PR! Apologies for the slowness oft he review.

@NullVoxPopuli
Copy link
Contributor

image

a bit less code

really underselling it there :p

@NullVoxPopuli
Copy link
Contributor

@transitive-bullshit, I like the work you've been doing with benchmarking here: #33

any chance you can run those tests on this branch? Thanks!!!

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.

3 participants