-
Notifications
You must be signed in to change notification settings - Fork 26
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
V4 API for run()
, call()
, spawn()
#912
Comments
Conversely, when using NestJS and Effection together, I often need to use 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 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 |
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 yield* apply(someThing, someThing.doStuff, ["Hello"]); If you end up opting for the "array as a first arg for |
I think it depends. The thing I liked about v3 So if all we are trying to do is align However, if in v4 we are moving to a world that is more similar to Lemme know if that doesn't make sense since this sort of relies on understanding how |
@neurosnap has an excellent point. When I first started reading the Effection docs, I drew a lot of parallels between Effection and |
Let's just keep it as That said, I do like using the array idea from redux saga as a way to pass |
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?
The text was updated successfully, but these errors were encountered: