Skip to content
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

[Typescript] Yield return value type safety #884

Closed
ssynix opened this issue Mar 27, 2017 · 35 comments
Closed

[Typescript] Yield return value type safety #884

ssynix opened this issue Mar 27, 2017 · 35 comments

Comments

@ssynix
Copy link

ssynix commented Mar 27, 2017

I'm ramping up on redux-saga and Typescript by working on a small project.

Something I've noticed is that the return value of yield effect(...) is always any. I'm not familiar enough with the platform to understand if this is a limitation of Typescript, or some missing type annotation in redux-saga/my code.

E.g.:

function selector(state: State) {
    return state.selected;
}

function* yieldNumber() {
    yield 1;
}

function* saga() {
    let num = yield call(yieldNumber); // num has inferred type of any
    let selected = yield select(selector); // selected: any

    // My workaround
    let num: number = yield call(yieldNumber); // works as long as I don't misrepresent the type
}
@ssynix
Copy link
Author

ssynix commented Mar 27, 2017

I believe this is a Typescript limitation, so I filed an issue on their end: microsoft/TypeScript#14883.

@ssynix ssynix closed this as completed Mar 27, 2017
@raduflp
Copy link

raduflp commented Dec 28, 2017

facing the same issue now that I'm trying to adopt Typescript.
@ssynix did you find a workaround/pattern to have type safety for the saga's return values?

@ssynix
Copy link
Author

ssynix commented Dec 28, 2017

@raduflp There's no workaround since it's lacking support at the language level. There are several threads mentioned in the issue above that discuss this feature, saying that it was blocked on boolean literal types and default template parameters (both now implemented), but generator yield type propagation still hasn't seen any traction in the past year.

The closest thing to a workaround is still what I wrote in the code sample above: you can manually assert the type yourself to make sure the rest of the code is typesafe, but there's nothing you can do to automatically detect if that original type-assertion is wrong.

@nicgirault
Copy link

nicgirault commented Jun 30, 2019

It seems that typescript 3.6 will finally bring support for strongly typed iterators: microsoft/TypeScript#31639

@Andarist
Copy link
Member

Andarist commented Jul 1, 2019

@nicgirault TS@3.6 will supposedly come with better support for generators, so this is probably something which we'll tackle then.

@PejmanNik
Copy link

Hi @Andarist Is there any in-progress task due to this issue? or help how I can make PR for that?
The microsoft/TypeScript#31639 is closed and it is available with next tag.
Thank you

@Andarist
Copy link
Member

@PejmanNik this is happening in #1892

@PejmanNik
Copy link

Thank you @Andarist 👍

@gilbsgilbs
Copy link
Contributor

@PejmanNik I don't want to disappoint you, but note that my PR won't be sufficient to fix this issue properly. Saga generators would indeed be strongly typed, yet yielded effects return types would still be any as in the OP example. As far as I know, TS 3.6 still won't be able to make TNext depend on what is yielded.

However, I have a PoC running locally proving that it should be possible to make a ESLint rule which would assert that:

  • You always provide an explicit type for yielded effects
  • The type you provide is consistent with the return type of the effect

I'll work on this ESLint rule when #1892 gets merged if I have some time. Sure having proper type inference built at the language level would be way better, but such ESLint rule would at least prevent most mistakes I think.

@PejmanNik
Copy link

Thank you @gilbsgilbs for your great work.
I have a crazy idea and want to know what do you think about it
after so many trials I found yield* can detect return type of function. so we can implement a custom version of each effect that must call with yield* and in that new custom function, we just handle type and then call real one with a yield. 🤷‍♂️😓

@gilbsgilbs
Copy link
Contributor

@PejmanNik Well, I'm not sure to follow. Do you have a code example?

As far as I know, in this case:

function* gen0() {
  yield* [1, 2, 3];
  return 'foo';
}

function* gen1() {
  const result = yield* gen0();
}

result has type "any".

@PejmanNik
Copy link

PejmanNik commented Jul 19, 2019

oh sorry @gilbsgilbs. I think this is crazy way but at last it work

function* callSafe<Fn extends (...args: any[]) => Generator>(fn: Fn, ...args: Parameters<Fn>)
{
  const result = yield call(fn,...args);
  return result as Parameters<ReturnType<Fn>["return"]>[0];
}

function* typeTest ()
{
  const result = yield* callSafe(fn);
}

result has real ReturnType<Fn>

I'm on Ts 3.6.0-dev.20190718 and the result of yield* gen0(); has type "string" for me.

@gilbsgilbs
Copy link
Contributor

gilbsgilbs commented Jul 19, 2019

This is very hackish, yet it makes sense and I think your approach is correct (typings-wise) and isn't a bug in TypeScript. I didn't realize yield delegation was improved in TS 3.6 and I made my test against TS3.5, but in TS3.6 we do actually get strong types with the same snippet.

Maybe this is something we could include in the TypeScript recipe or even in a third-party library 🤔 . Well done.

Edit: It may be quite easy to confuse though (in similar scenarios to the one described in #1286 (comment) section "Generic types and Sagas"). We'll have to test this out.

@PejmanNik
Copy link

Thank you @gilbsgilbs
Yes I got the idea from there 😁

If you and other maintainers of saga think it can help. I can work on PR or a small third-party library.
What do you think?

@gilbsgilbs
Copy link
Contributor

I'm not a maintainer of Redux-Saga 😉 . Just a happy user who is looking for improved type-safety and trying my best to contribute towards this direction. Since it's a TS-specific hack, I don't think it has its place in the main repo.

@PejmanNik
Copy link

PejmanNik commented Jul 19, 2019

so many of your comments help me. Thanks for having TS user back 😄
Yes, I agree. I just wonder how others think about making this as a library.

@danielnixon
Copy link

@PejmanNik I just threw this together, it might serve as a starting point.

https://github.com/agiledigital/typed-redux-saga

@parkerault
Copy link

parkerault commented Aug 26, 2019

@PejmanNik @danielnixon has kindly done the work to get the library you proposed started. As of this writing typed-redux-saga is an active repo with 11 commits, and redux-saga-safetype has a single commit with an empty README.md. He didn't steal your idea, he's giving you a foundation to build on. Why don't you help out instead of condemning him contributing to the community?

@SantoJambit
Copy link

Shouldn't this be part of this library rather than a separate library?

@jmrossy
Copy link

jmrossy commented Aug 5, 2020

Return typings worked for me with @danielnixon's typed-redux-saga library but it would be great to get this integrated into the main lib.

@SantoJambit
Copy link

#2053 is about exactly that, but it's stuck in code-review. @Andarist seems to be busy. Not sure if there are other maintainers who could merge it.

@egorovsa
Copy link

Hey guys, three years since the topic was started...
We still have inferred type of any.
typed-redux-saga is good, yes but it's not serious whereas big library has typing that does not work.

or may I have an example when it works good?

@OnkelTem
Copy link

OnkelTem commented Mar 2, 2021

Four years in 25 days already, @egorovsa ! But that's not fair, we should start counting at least from the release of TS 3.6 - which is 1.5 years.

I would like to thank everybody trying to have it done, guys. It was funny to discover that yield* "magically" works where yield does not. IIRIC 4.2.2 now is the current version and yet there's no a solution for typing yields. Alas, it's not that easy.

@edshav
Copy link

edshav commented Mar 4, 2021

// My workaround
// Doesn't work with functions that return a generic type like Promise<SomeType>
    let num: ReturnType<typeof yieldNumber> = yield call(yieldNumber);

@saswat3115
Copy link

This is a serious issue. I spent a whole day to fix this. I added a * just next to yield as per typescript suggestion but my whole saga stopped working. One way is to disable typescript chekc strcit mode off untill this is fixed

@OnkelTem
Copy link

OnkelTem commented Mar 4, 2021

@saswat3115 Honestly, I ended up just putting dozens of // @ts-ignore after upgrading to TS 4.2 the day before yesterday. That's because things like const token = yield select(getToken) just don't work anymore.

As for the stars — that can only work with https://github.com/agiledigital/typed-redux-saga , as the underlying stuff need to be changed too. But it didn't work for me as well because I was going to use its Macro mode and that didn't work for some reason.

@parkerault
Copy link

I've been using typed-redux-saga in production for two years now, and I haven't had any problems with it at all. I highly recommend switching instead of bending over backwards trying to make redux-saga type safe.

@OnkelTem
Copy link

OnkelTem commented Mar 9, 2021

@parkerault Sounds encouraging! Do you use starred yields or macro?

@parkerault
Copy link

@parkerault Sounds encouraging! Do you use starred yields or macro?

I started using it before the macro was available, so I've always used yield*. There are some cases where I've had to fall back to the untyped effects for reasons that I can't remember off the top of my head, which isn't ideal since every team member needs to understand the difference between the two libraries. However, the benefits still outweigh the drawbacks in my opinion. Regardless, I haven't encountered any bugs or unexpected behavior while using typed-redux-saga in production.

@harvey56
Copy link

I installed typed-redux-saga, and I get the following TS error message upon compilation :

undefined TypeScript error in undefined(undefined,undefined): Cannot find global type 'Array'. TS2318

Not sure what to do with this. I guess I will do what @OnkelTem did. I will add // @ts-ignore everywhere in my sagas, because everything was working fine until I installed VS code updates.

@Kepro
Copy link

Kepro commented Mar 15, 2021

@harvey56 did you try to restart webpack?

@parkerault
Copy link

parkerault commented Mar 15, 2021

undefined TypeScript error in undefined(undefined,undefined): Cannot find global type 'Array'. TS2318

@harvey56 are you compiling from within VS Code? if you started getting an error when you upgraded VS code, it may be the version of typescript that it uses internally that's causing the problem. The typed-redux-saga repo has ^4.2.3 in its devDependencies, so try putting that in your package.json, rebuild node_modules, then change the typescript version VS code is using by running typescript: select typescript version with ctrl/cmd+shift+p. Can you paste a snippet of the code that causes this error?

@akunal1
Copy link

akunal1 commented Mar 25, 2021

Use the response type of your api .

For me, I am using Apisauce. You can refer below code.

import { ApiResponse } from 'apisauce';
const response : ApiResponse<any,any> = yield call(api.machinesList, action.params);

@akili862002
Copy link

Hi there, I was crazy with this thing before. Then I found that If I add an interface for responce..., that warning was gone.
I'm using graphQL, so this is what I did:

interface IResponse {
    data?: any;
}

export function* getStreets(payload: any) {
    const variables = payload.payload;
    const response: IResponse = yield call(services.getStreets, variables);
    const { getStreetByProvinceAndDistrict } = response?.data?.result || {};
   // ......
}

@parkerault
Copy link

Hi there, I was crazy with this thing before. Then I found that If I add an interface for responce..., that warning was gone.
I'm using graphQL, so this is what I did:

interface IResponse {
    data?: any;
}

export function* getStreets(payload: any) {
    const variables = payload.payload;
    const response: IResponse = yield call(services.getStreets, variables);
    const { getStreetByProvinceAndDistrict } = response?.data?.result || {};
   // ......
}

That will work, but the problem with doing it that way is if someone changes the signature of services.getStreets to return IFoo, any code downstream of the saga will still think it has an IResponse. You lose the benefits of type inference and won't catch potential runtime errors. That's the reason @danielnixon created typed-redux-saga. 🙂

darekkay added a commit to darekkay/dashboard that referenced this issue May 22, 2021
brunolemos added a commit to devhubapp/devhub that referenced this issue Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests