Polyfill example effect

[Phoscur]

I understood this is just a basic (naive) example, but it would be great to see it working anyways!
While I still need to wrap my head around how Signals (and the subtle stuff) work, I'd like to ask you to help me with this:

Let's fix the example from the readme, it's only logging "even" once on Node21:

import { Signal } from "signal-polyfill";
let needsEnqueue = false;

const w = new Signal.subtle.Watcher(() => {
  if (needsEnqueue) {
    needsEnqueue = false;
    queueMicrotask.enqueue(processPending); // should be just queueMicrotask(processPending)? - it's never called though
  }
});

function processPending() {
  needsEnqueue = true;
    
  for (const s of w.getPending()) {
    s.get();
  }

  w.watch();
}

export function effect(callback) {
  let cleanup;
  
  const computed = new Signal.Computed(() => {
    typeof cleanup === "function" && cleanup();
    cleanup = callback();
  });
  
  w.watch(computed);
  computed.get();
  
  return () => {
    w.unwatch(computed);
    typeof cleanup === "function" && cleanup();
  };
}

const counter = new Signal.State(0);
const isEven = new Signal.Computed(() => (counter.get() & 1) == 0);
const parity = new Signal.Computed(() => isEven.get() ? "even" : "odd");

effect(() => console.log(parity.get())); // Console logs "even" immediately.
setInterval(() => counter.set(counter.get() + 1), 1000); // Changes the counter every 1000ms.

// effect triggers console log "odd"
// effect triggers console log "even"
// effect triggers console log "odd"
// ...

[EisenbergEffect]

Apologies for the confusion. There was a typo in that example. It has been fixed in the repo's readme, just not updated on NPM yet. Here's a link:

https://github.com/tc39/proposal-signals/blob/main/packages/signal-polyfill/readme.md

@littledan @NullVoxPopuli How do we want to handle releasing polyfill updates?

[Phoscur]

Oh needsEnqueue needs to be true at the start, ofc!

Here is a typed version of the effect function, did I get that right, these are called again (curried) to clean up?

type CleanUp = () => void;
export function effect(callback: () => void | CleanUp): CleanUp {
  let cleanup: void | (() => void);

  const computed = new Signal.Computed(() => {
    typeof cleanup === 'function' && cleanup();
    cleanup = callback();
  });

  w.watch(computed);
  computed.get();

  return () => {
    w.unwatch(computed);
    typeof cleanup === 'function' && cleanup();
  };
}

[NullVoxPopuli]

How do we want to handle releasing polyfill updates?

maybe we should extract to its own Repo so we can have an independently maintained changelog? (I can set up all that automation, etc)

but it would be great to see it working anyways!

@Phoscur there is an implementation here: https://github.com/proposal-signals/signal-utils/blob/main/src/subtle/microtask-effect.ts

And demo'd here: https://jsbin.com/safoqap/edit?html

[ljharb]

A separate repo also has the benefit of separating out issues about the implementation from issues about the specification and design.

[Phoscur]

@Phoscur there is an implementation here: https://github.com/proposal-signals/signal-utils/blob/main/src/subtle/microtask-effect.ts

Oh, but that one is different, it leaves out any possibilty to clean up, on purpose? Any reason you don't want (me) to add it?

[NullVoxPopuli]

, it leaves out any possibilty to clean up, on purpose?

how do you mean? it returns a cleanup function that you'd have to call.

Any reason you don't want (me) to add it?

I'd be more than happy to receive PRs for alternate effect implementations!

[Phoscur]

, it leaves out any possibilty to clean up, on purpose?

how do you mean? it returns a cleanup function that you'd have to call.

Comparing https://github.com/proposal-signals/signal-utils/blob/main/src/subtle/microtask-effect.ts and https://github.com/tc39/proposal-signals/blob/main/packages/signal-polyfill/readme.md then the first one (TypeScript) is leaving out the clean-up procedure the callback can provide as a (curried, optional) return: effect(callback: () => void | CleanUp)

I saw you added the first part (clean-up return for effect) recently, removing some of the extensive comments concerning the memory leakage. So with this we could remove the other comments?

The question remains what other pitfalls this "naive" implementation hides? The readme says framework authors are supposed to implement this specifically for batching. So I guess there is some overhead here?
Maybe it could be worth to annotate why queueMicrotask is used here and alternatives (e.g. comparing to setTimeout(x)).

Any reason you don't want (me) to add it?

I'd be more than happy to receive PRs for alternate effect implementations!

Yeah, it would be really good to see a more advanced scheduler examplewise too.