tp / tsaga

Typesafe and lightweight way to write Redux-connected functions with asynchronous side-effects that are easily testable.
Apache License 2.0
8 stars 1 forks source link

Features/effects #23

Closed HenriBeck closed 5 years ago

HenriBeck commented 5 years ago

Not sure what the problem with the inferred typings of your Effect implementation is, but I got them working.

HenriBeck commented 5 years ago

Try playing around the src/test-app/sagas/user-saga.ts on line 61. It should require the type number.

The definition of an effect is in the src/lib/types.ts

tp commented 5 years ago

The problem in OMS is that I can't write a helper that generates an effect which:

I can get either one of them working, but not both at the same time :(

tp commented 5 years ago

Yeah, that basic effect with a wrapped run is nice, except for the fact the inner function captures the parameters via closures and is created anew for each instance. So we can't easily provide a mock for it, right?

Requiring the number parameter should be fixable, that is already working in some of the other approaches.

HenriBeck commented 5 years ago

Mocking would be the same as with other run functions.

Saga:

await run(callBapi, params);

Test:

withRun(callBapi, fakeResponse)

Right now it requires the number param; the inferred arguments should work.

Why would a new function be created for ea

Yeah, that basic effect with a wrapped run is nice, except for the fact the inner function captures the parameters via closures and is created anew for each instance. So we can't easily provide a mock for it, right?

Which instance?

tp commented 5 years ago

But run(callBapi, params) will never return the correct T, no matter how we implement it, due to https://github.com/Microsoft/TypeScript/issues/29638

So either we have to reorder the params (👎😅) or find a way to wrap it while still allowing for tests.

(For the time being it would still have to be called as run(callBapi(request)) (so that callBapi can return the correct response type)).

tp commented 5 years ago

Which instance?

I meant each instance of the effect. Or are they created only once? Will have to stash my stuff and check this out in detail, just reading the diffs it wasn't clear to me.

HenriBeck commented 5 years ago

I meant each instance of the effect. Or are they created only once? Will have to stash my stuff and check this out in detail, just reading the diffs it wasn't clear to me.

The effects are only created once when the app loads.

HenriBeck commented 5 years ago

But run(callBapi, params) will never return the correct T, no matter how we implement it, due to Microsoft/TypeScript#29638

So either we have to reorder the params (👎😅) or find a way to wrap it while still allowing for tests.

(For the time being it would still have to be called as run(callBapi(request)) (so that callBapi can return the correct response type)).

The return typings are also working on my end. Try returning a string from the effect and call on that then concat.

EDIT: Forgot that concat is actually a method on a string 🤦‍♂️ try push

tp commented 5 years ago

@HenriBeck Usually return types will work, it's just the generic don't get correctly assembled. If a function a simple / static return type it'll be fine

HenriBeck commented 5 years ago

Ahh, I think I will need to read further into OMS and the backbone package to understand how it all comes together. There should be a solution to this problem.

tp commented 5 years ago

Yep, I can give you an intro tomorrow. Probably makes sense to spend the time to come to a solution that works correct for that case since it's so widely use already and coming to the app soon :)