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

Calling .watch inside a Computed callback #211

Open
prophile opened this issue Apr 29, 2024 · 1 comment · May be fixed by #223
Open

Calling .watch inside a Computed callback #211

prophile opened this issue Apr 29, 2024 · 1 comment · May be fixed by #223

Comments

@prophile
Copy link
Contributor

Consider the following code:

let watcherB
const state = new Signal.State(false)
const computed = new Signal.Computed(() => {
  if (state.get()) {
    watcherB.watch(computed)
    return 1
  } else {
    return 0
  }
})

computed.get()
watcherB = new Signal.subtle.Watcher(() => {
  console.log("B notified")
});
const watcherA = new Signal.subtle.Watcher(() => {
  console.log("A notified")
});
watcherA.watch(computed)
state.set(true)

During the execution of its callback, computed makes itself watched by watcherB. The semantics as they currently stand mean that both by the spec and the polyfill at f7c550b, only "A notified" is logged. This is probably right, but is also potentially quite surprising, and may be best spelled out explicitly?

I foresee browsers doing creative strategies to optimise Signal as long as the semantics are compatible with the "push-dirty pull-execute" semantics spelled out; I worry that a case like this might end up changing behaviour there unless it's more explicitly spelled out.

@littledan
Copy link
Member

What is it that you'd like to be spelled out? I don't see how "B notified" would ever run here.

To align browser behavior: let's add more tests for this stuff!

@prophile prophile linked a pull request May 15, 2024 that will close 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

Successfully merging a pull request may close this issue.

2 participants