Open cowboyd opened 2 months ago
it's annoying in that we don't really ever use this.
Conversely, when using NestJS and Effection together, I often need to use this
, which can be quite inconvenient.
import { globalScope } from '..'
@Controller('receiver')
export class ReceiverController {
@Inject()
private readonly passport!: Passport
@Inject()
private readonly pushService!: PushService
@Delete()
public [`@Delete()`](): Promise<void> {
return globalScope.run(function*(this: ReceiverController) {
const receiver = yield * this.pushService.getClaimerReceiver(this.passport.id)
if (null === receiver) {
return
}
yield * this.pushService.deleteReceiver(receiver)
}.bind(this))
}
}
@iplaylf2 Thanks so much for your comment. This is a great data point!
I just came across Effection last week and am loving it! Just my two cents on this: you could use an approach similar to Redux-Saga and allow the first arg to call
be function or an array where the first element of the array is the thisArg
and the second is the function:
class SomeClass {
public async doStuff(someArg: string): Promise<void> {
// Stuff being done...
}
}
const someThing = new SomeClass();
yield* call([someThing, someThing.doStuff], "Hello");
That would make it easier for users to upgrade to the new API and wouldn't require a lot of extra code on your part to check if the call
function has a this
context.
Glad to hear you like it @mikerourke! We've got a lot of fun stuff coming down the pike too.
This is a really interesting approach that I hadn't considered, but it seems really nice! Is there any downside in your experience? /cc @neurosnap
@cowboyd I've been using Redux-Saga in several projects for the last 4 years, and I haven't encountered any issues with that API for call
. It's probably worth mentioning that Redux-Saga also provides an apply
effect that has a similar API to Function.prototype.apply
that would look like this in Effection:
yield* apply(someThing, someThing.doStuff, ["Hello"]);
If you end up opting for the "array as a first arg for call
" approach when you need a this
context, it might be nice to provide apply
as a convenience wrapper around the call([thisArg, fn], ...args)
API.
The question is 1) is this desirable?
I think it depends. redux-saga
call
function type signature is pretty complicated since it can handle so many use cases -- including properly handling this
. All of this is stemmed from the fact that the end-user doesn't call the function being passed into call
, rather, the saga runtime handles calling all functions. The reason why this was preferred is because of the way saga handled side-effects: the end-user doesn't have to deal with managing side-effects because user functions don't actually call them.
The thing I liked about v3 call()
is that we didn't have to deal with these use cases at all since the end-user does call the function itself: call(() => response.json());
So if all we are trying to do is align Function.prototype.call
with v3 call()
then I would rather argue we just change the name of call
to something specific to what it is trying to accomplish in effection.
However, if in v4 we are moving to a world that is more similar to redux-saga
where the effection runtime run()
evaluates a bunch of yielded json objects and then converts them into function calls / activating side-effects, then we will most likely have to do it the way suggested above.
Lemme know if that doesn't make sense since this sort of relies on understanding how redux-saga
works.
@neurosnap has an excellent point. When I first started reading the Effection docs, I drew a lot of parallels between Effection and redux-saga
. But I'm just getting started with Effection, so I could be completely off base.
One of the things happening in
v4
is that we're aligningcall()
to be more similar toFunction.prototype.call()
This would imply that the signature forcall()
would be:The question is 1) is this desirable? and 2) what should we do with the other "call-like" functions in Effection:
run()
andspawn()
. We could align them like this;On the one hand, this has a nice alignment with vanilla js, but on the other, it's annoying in that we don't really ever use
this
. Still, this is supposed to feel like native JS building blocks, so I'm not sure what the best way to go is.On the other hand, I've been toying with the idea of deprecating
scope.run()
and replacing it by passing scope as a parameter torun()
This would re-enforce the idea that this is an entry point from javascript and something you should do with care.Which is better?