[RFC] Represent notify/refresh events with signal utility
Closed this issue · 15 comments
UPDATE: Proposed utility can be viewed in this PR (feel free to continue with feedback): #277
We can run into situations where we want to imperatively indicate that something has happened in order to trigger a re-computation of a signal or an effect. This is the type of event that might have typically been represented by Subject<void>
.
NOTE: This is not intended to replace the role of a Subject
generally, thanks to @brandonroberts for pointing out important context to add here: if the notifier signal were triggered three times, it doesn't necessarily mean that an effect
would be triggered three times because they would (if they were called in succession) be combined into the same tick
, so it would only be triggered once. This may be undesirable behaviour. The purpose of this utility is to provide the ability to trigger some thing when something happens, which is just one particular case that might have been handled by a Subject.
The core idea for this came originally from @alxhub who recommended, on the removal of mutate
from signals, that a "version" signal could be used to indicate that a map has been mutated - essentially just incrementing a signal by one.
This proposal deals with the map mutation situation, but also generally any situation where you want to trigger something when something else happens. The usage would look something like this:
EDIT: Updated example usage to incorporate suggestions:
// create the notifier
myTrigger = createNotifier();
// trigger the notifier
myTrigger.notify();
// reacting to the notifier
myThing = computedAsync(async () => {
this.myTrigger.listen() // signal referenced to trigger computation
// do stuff async stuff and return
});
// or in an effect
constructor(){
effect(() => {
this.myTrigger.listen();
// do something
});
}
The implementation for this utility that I created looks like this:
import { Signal, computed, signal } from '@angular/core';
export function notify(): Signal<number> & {
notify: () => void;
init: Signal<boolean>;
} {
const sourceSignal = signal(0);
const readonlySignal = sourceSignal.asReadonly();
Object.defineProperties(readonlySignal, {
notify: {
value: () => sourceSignal.update((v) => v + 1),
},
init: {
value: computed(() => readonlySignal() === 0),
},
});
return readonlySignal as Signal<number> & {
notify: () => void;
init: Signal<boolean>;
};
}
NOTE: I have also added an init
signal so that if you only want to react to the notifier being explicitly triggered (not on init as well) then you can reference the init signal and check that it is false
.
@eneajaho also created a similar utility that he has been using:
function triggerer() {
const s = signal(0);
Object.defineProperties(readonlySignal, {
tick: { value: () => s.update((v) => v + 1) }
});
return s as Signal<number> & {tick: () => void};
}
Feel free to share any thoughts you have on how this should be implemented and also feel free to leave your thoughts on the naming, for example do you prefer something like:
notify
ornotifier
that is triggered with.notify()
triggerer
that is triggered with.tick
or.trigger()
- or something else
Hello,
I like this idea and it's something I've discussed with @eneajaho like months ago, but in a different flavor.
My only main feedback is the name of the function that constructs this mechanism, nofify()
is very generic (what is notify?).
Imagine that we have to re-run a computed
(async) because we want to refresh it manually (which is one of the use cases).
I'd declare it like:
const requestAbcManualRefreshNotifier = createManualNofitierSignal()
Why this name?
- We're creating a signal
- It acts as a notifier
- It purpose is to be manual triggered
About the init
thing, that's something to be thought about more.
And I'd change it to something like:
myThing = computedAsync(async () => {
requestAbcManualRefreshNotifier.listen();
});
Thanks Renato, the naming should definitely be considered more (for me I just needed to use it in a project and didn't put too much thought into the naming initially)
I like the idea of making it more explicit, but what about:
createNotifier()
and I also like the idea of something like a listen()
method (whether it ends up being called listen
or something else) which makes the role more clear within the computed/effect rather than just looking like a random function call just dangling there.
- Very needed of this
- computedAsync is usually triggered by props signal in this way you added one more, maybe name should be ReCompute Trigger and part of options argument
Some ideas:
export function notify(): Signal<{}> & {
notify: () => void;
isPostInit: boolean;
} {
const sourceSignal = signal({});
const readonlySignal = sourceSignal.asReadonly();
let isPostInit = false;
Object.defineProperties(readonlySignal, {
notify: {
value: () => {
sourceSignal.set({});
isPostInit = true;
},
},
isPostInit: {
get: () => isPostInit,
},
});
return readonlySignal as Signal<{}> & {
notify: () => void;
isPostInit: boolean;
};
}
I just realised the init
property would be unnecessary as long as a version number that starts with 0
is used. We could just do this:
effect(() => {
this.myTrigger.listen();
// do stuff
})
For normal cases, and if we don't want to trigger on init we could do:
effect(() => {
if(this.myTrigger.listen()){
// do stuff only after init
}
})
When the version is 0
the if
will fail, once it has been triggered at least once it will always be true
I just realised the
init
property would be unnecessary as long as a version number that starts with0
is used. We could just do this:effect(() => { this.myTrigger.listen(); // do stuff })For normal cases, and if we don't want to trigger on init we could do:
effect(() => { if(this.myTrigger.listen()){ // do stuff only after init } })When the version is
0
theif
will fail, once it has been triggered at least once it will always be true
It can be even more simpler than that (I believe).
Could try to control if you expect the listen() to emit or not for the init, so I believe that something like this could be done:
createNotifier({
emitImmediatelly: true / false
})
I wonder if this could be achieved since signals must always emit at least once.
@joshuamorony I thought you need init
for better code clarity :)
@renatoaraujoc the first value will be read anyway, just because effect()
is initialized and reads its signals for the first time.
@joshuamorony I thought you need
init
for better code clarity :)@renatoaraujoc the first value will be read anyway, just because
effect()
is initialized and reads its signals for the first time.
@e-oz sometimes you're so attached to RxJS that you forget some stuff :P
I've create an initial implementation in a draft PR incorporating the feedback here - feel free to continue suggesting any specific changes you'd like to see on the PR
how we are do it this recompute
domain = input.required<string>();
records = computedAsync(() => this.#domainService
.getGenericDnsRecords(this.domain(), DnsRecordType.A)
.pipe(
mapArray((r: DnsRecordA) => { return new AGui(r) }),
repeat({ delay: () => this.#aStateService.refetch }),
catchError((_err) => { this.#uiService.showErrorFlashMessage("Napaka pri pridobivanju zapisov"); return of(null) })
));
and I for now it is ok, but this new idea I like it very much
I like the createNotifier
, notify
and listen
naming, pretty straightforward (much more than new Subject()
, .next
and subscribe
at least, even though you said it wasn't to replace it I think it's kinda comparable).
One question, you said it was intended to be used inside effect/computed but could we use it anywhere still? Like in any function that we want to rerun.
Another possible surface API implementation could be createListener
instead of having the listen
property inside the notifier.
componentInitNotifier = createNotifier();
modalInitNotifier = createNotifier();
export class CompA {
constructor() {
componentInitNotifier.notify()
}
}
export class CompB {
constructor() {
componentInitNotifier.notify()
}
openModal() {
modalInitNotifier.notify();
}
}
export class CompC {
constructor() {
componentInitNotifier.notify()
}
}
// some service
export class TrackingService {
#trackUsefulEvents = createListener(
[componentInitNotifier, modalInitNotifier],
() => {
// perform tracking
})
}
I think it's read better to have the deps array right from the start.
On the other hand, maybe having the listener following one each other could allow more synchronous code to be triggered.
constructor(){
effect(() => {
this.myTrigger.listen();
// do something
this.myOtherTrigger.listen();
// do something else
});
}
Will the code execute synchronously? I guess it depends on what occurs, but that could be neat.
Also, do you consider the possibility to pass a data through notify
, for example componentInitNotifier.notify('CompA')
that could be retrieve like so data = componentInitNotifier.listen()
or something.
I've create an initial implementation in a draft PR incorporating the feedback here - feel free to continue suggesting any specific changes you'd like to see on the PR
Commented on it!
@sysmat I haven't used computedAsync
this way but I'm pretty sure it would work to reference the notifier and then just return the observable, but personally if I was already using an observable anyway I'd just next a subject rather than use the notifier signal
@KevTale no the mechanism relies on the signal value changing and anything referencing that signal recomputing as a result, so would have to be in computed/effect. For the computed/effect it is essentially going to be all or nothing - if any of the notifiers are triggered then the entire thing is going to run, the only thing we can really differentiate on within the computation is the initialised/not initialised state
and there is probably more stuff you could play around with here but personally I prefer to keep this particular utility on the simple side - signal value changes -> trigger re-computation wherever referenced. I think if we do thing like notifying with specific data we are getting into dangerous territory. That's sort of what I was referring to in the little warning/disclaimer - let's say we trigger a notify
with valA
and then valB
you might write code expecting to receive both of those "events" but you might end up only ever getting valB
.
If we want to force the tick we can also do:
setTimeout(() => tick())
And have this as a method called forceNotify inside the utility.
Do you think there are any use cases that make this compelling? I do worry it might stray too far into attempting to represent events with signals (versus just the small/isolated use case of triggering a computation) - another thing to consider is that of notifying with the same data twice - again people might expect it to trigger the effect twice, but if the data hasn't actually changed then it won't run, so then we'd have to add in some other workaround for that (which is doable, but to me feels like it's getting into weird territory)