WebReflection/usignal

reconsider .then()

Closed this issue ยท 17 comments

Hi Andrea

However much I like "then()", here it can lead to problems.
Or am I missing a way to return a signal from an asynchronous function?

async function getSignal(){
  cons value = await fetchValue();
  return signal(value)
}

const s4 = await getSignal();

console.log(s4.value) // undefined
console.log(s4) // the value

I don't understand the use case for this ... that async function is not an effect, what are you after there? then() is meant to return signals containing promises in them, not the other way around. Closing until it's clear what you are doing.

signal(fetch('url')); // ๐Ÿ‘Œ
fetch('url').then(v => signal(v)); // ๐Ÿคท

This is perhaps an understandable use case.

async function insert(name) {
    const lastInsertId = await query(`INSERT INTO x SET name = ${name}`);
    const nameSignal = signal(name);
    effect(()=>{
        query(`UPDATE x SET name=${nameSignal} WHERE id=${lastInsertId}`);
    })
    return nameSignal;
}

signal = await insert('Joe'); // should await for insert, but return a signal
signal.value = 'John';

I mean no async function is able to return a signal.

query(`INSERT INTO x SET name = ${name}`);

either there's an error there, or that query fails and it's shenanigans prone security wise, same goes for the query inside the effect.

That being said, a signal can contain a Promise, primary use case, so this one would be different, as you know the name to insert already:

async function insert(signal) {
    const lastInsertId = await query(`INSERT INTO x SET name = ${signal.peek()}`);
    effect(()=>{
        query(`UPDATE x SET name=${signal} WHERE id=${lastInsertId}`);
    })
    return signal;
}

let name = await insert(signal('Joe')); // should await for insert, but return a signal
name.value = 'John';

OK, maybe I got the issue there ... the returned signal resolves ... bummer, but both cases are valid to me, and used already.

How about this?

async function insert(signal) {
    const lastInsertId = await query(`INSERT INTO x SET name = ${signal.peek()}`);
    effect(()=>{
        query(`UPDATE x SET name=${signal} WHERE id=${lastInsertId}`);
    })
    return {
      get value() { return signal.value },
      set value(_) { signal.value = _ }
    };
}

???

not the best answer, but the first that comes to mind ... the current signature is kinda sailed, unless I release a major with reasonable good enough reasons and use cases why current feature is not welcome ... arguably, the current signature indeed doesn't fully allow a value setter, so I need to think about it!

P.S. what's Preact doing in here? happy to hear from their experience around this topic too

either there's an error there...

This is just minimal dummy code, but maybe I should write it properly for ChatGPT ๐Ÿคฃ

P.S. what's Preact doing in here? happy to hear from their experience around this topic too

It seams Preact-Signals are not thenables

it's possible I made them so due computed, see https://github.com/WebReflection/usignal/blob/main/test/test.js#L16-L21

I am thinking effectively signal(fetch('url')) makes very little sense, as that's a read-only signal in meaning and intent ... but computed(() => signal.value + fetch('url')) which is indeed read only, makes sense as awaitable, although that could be inferred by computed(async () => stuff) which should already work out of the box ... OK, I am fairly convinced now this was a mistake on my side: do you have any other use case to think about? should then just disappear as helper as it makes any signal effectively read-only ?

although that could be inferred by computed(async () => stuff) which should already work out of the box

it doesn't, as computed won't resolve at distance async callbacks ... once again, what's Preact position in here? ๐Ÿค” (guessing computed are not thenable neither ... yet it's an interesting after-thought to me)

I am fairly new to signals and have never come across a usecase for await signal.
But if the benefit is only the saved ".value", I would do without it.

that was the overwhelming poll result: https://twitter.com/WebReflection/status/1571400086902476801

admittedly, neither me nor others presented use cases for such scenario, so it was "community driven" and it looks like the community didn't think enough about it ๐Ÿ˜…

I was also for "yup" ๐Ÿ˜ฌ

cool, so it's your fault ... ๐Ÿคฃ what are you suggesting? dropping the then seems to fix them all and it's a breaking change that should go major (I don't even remember if this is major already) ... alternatives to then ??? I can't see myself any

now that I've looked at the history here ... you helped improving then there ... do you you remember why you did so? if it was before realizing it was a mistake, then I'm OK dropping that, specially as we all agree at this point this was my mistake https://twitter.com/_developit/status/1628868903404290049

Yes, I am new to signals and I have only just noticed this problem.

You could maybe leave the function (then) in for the next version and output a console.error('deprecated') to draw attention to the change.

but I want to be able to do your example now ๐Ÿ˜

I've started major updates to side/smaller libraries but I'll drop this thenability nonsense in here too.

OK, v0.9 drop .then(...) all together and it breaks whoever was using it before:

73c6abf

this is a young module and I think .then wasn't too popular so I think it's OK this way and being semver compat I don't even feel bad for it ๐Ÿ˜‡