getify / CAF

Cancelable Async Flows (CAF)
MIT License
1.34k stars 46 forks source link

Typescript typings file :) #30

Closed noomorph closed 1 month ago

noomorph commented 2 years ago

Hi, is there any plan to have one? 👋

getify commented 2 years ago

I would consider/accept a contribution of one, but I don't plan to make one (I'm not competent enough at TS).

I think I've heard that things like generators are really hard for TS to type properly. But that's just anecdotal and I might be totally wrong about that.

mustafa519 commented 1 year ago

@getify

I attempted to reproduce the codebase on my local environment, but I ultimately gave up. While I am comfortable writing code in ES6+ with many ESLint rules, I am not an expert in JavaScript and I found the codebase confusing. Specifically, the use of var declarations and parameter reassignments made it difficult for me to follow the flow of the code. Like, I was unsure whether I needed to add let or const to free up memory.

If it is possible to reproduce the codebase in ES6, I would be willing to try to add the necessary Typescript types. However, I need the codebase to be written in a way that is easier for me to understand and work with, as a noob JS developer.

getify commented 1 year ago

If it is possible to reproduce the codebase in ES6

The codebase is already ES6 (actually, at least ES2018, in that it uses async generators). For example, here's a let keyword being used where I believe it should be used:

https://github.com/getify/CAF/blob/a2b12de6ba7357fc25bcaa36b43c47f017878fe4/src/caf.js#L73

Specifically, the use of var declarations

I know there's a lot of cult wisdom in the "modern JS" way of thinking, followed by lint rules, that say var should never be used. I completely and totally disagree. There's nothing about var that's "non-ES6".

Put simply, I use var for any declaration that's at the top-most scope of a function (or module). I use let for any declaration that appears inside a block (such as if or for loop). One little exception is when I have a declaration inside a try that I want to access outside that block, I use a var there.

More info on how I advocate using var / let / const: https://github.com/getify/You-Dont-Know-JS/blob/2nd-ed/scope-closures/apA.md#the-case-for-var

I was unsure whether I needed to add let or const to free up memory.

That's not the appropriate reason to decide between these different declaration types. See link above.

However, I need the codebase to be written in a way that is easier for me to understand and work with

I'm sorry you found the codebase too confusing. I wish my code were considered readable by folks. IMO it's much more readable than most codebases I see others write. But I appreciate that you may disagree.

In any case, I won't be rewriting the codebase. The code style I use here is how I've always written OSS code and how I've always taught JS code.

noomorph commented 1 year ago

Oh, thanks for reminding me about the existence of this issue :laughing:

I'll leave it here for now (spent a couple of minutes with ChatGPT):

declare module "caf" {
  export class CancelToken {
    controller: AbortController | null;
    signal: AbortSignal;
    constructor(controller?: AbortController);
    abort(reason?: any): void;
    discard(): void;
    // _trackSignalReason(reason: any): void;
  }

  export function CAF<T, A extends any[]>(
    generatorFn: (signal: AbortSignal, ...args: A) => AsyncGenerator<any, T, any>
  ): (...args: [CancelToken | AbortSignal, ...A]) => Promise<T>;

  export function cancelToken(): CancelToken;

  export function delay(
    tokenOrSignal: CancelToken | AbortSignal | number,
    ms?: number
  ): Promise<string>;

  export function timeout(
    duration: number,
    message?: string
  ): CancelToken;

  export function signalRace(signals: AbortSignal[]): AbortSignal;

  export function signalAll(signals: AbortSignal[]): AbortSignal;

  export function tokenCycle(): () => CancelToken;
}

I (or someone else) might take it from here and finish it. I'm a bit busy currently but I hope to find some time in the nearest 2-3 months.

mustafa519 commented 1 year ago

If it is possible to reproduce the codebase in ES6

The codebase is already ES6 (actually, at least ES2018, in that it uses async generators). For example, here's a let keyword being used where I believe it should be used:

https://github.com/getify/CAF/blob/a2b12de6ba7357fc25bcaa36b43c47f017878fe4/src/caf.js#L73

Specifically, the use of var declarations

I know there's a lot of cult wisdom in the "modern JS" way of thinking, followed by lint rules, that say var should never be used. I completely and totally disagree. There's nothing about var that's "non-ES6".

Put simply, I use var for any declaration that's at the top-most scope of a function (or module). I use let for any declaration that appears inside a block (such as if or for loop). One little exception is when I have a declaration inside a try that I want to access outside that block, I use a var there.

More info on how I advocate using var / let / const: https://github.com/getify/You-Dont-Know-JS/blob/2nd-ed/scope-closures/apA.md#the-case-for-var

I was unsure whether I needed to add let or const to free up memory.

That's not the appropriate reason to decide between these different declaration types. See link above.

However, I need the codebase to be written in a way that is easier for me to understand and work with

I'm sorry you found the codebase too confusing. I wish my code were considered readable by folks. IMO it's much more readable than most codebases I see others write. But I appreciate that you may disagree.

In any case, I won't be rewriting the codebase. The code style I use here is how I've always written OSS code and how I've always taught JS code.

Don't misunderstand me, I do appreciate that you have fixed a very big problem as your project which is open-source. And, I really respect your efforts for JS.

I don't think codebase is entirely confusing or has bad readability. I am not good enough at JS to judge someone's project. Just confusing for the folks who have learned after ES6 and don't know the past of JS. I really don't know it well yet, like your other repo's purpose of the name. So, the problem was mostly about me, not about the codebase.

Sorry, if it was disrespectful to your efforts, I didn't mean that.

@noomorph you're lifesaver. Just in time. Thank you for sharing!

noomorph commented 1 year ago

@mustafa519 I'm not sure it will work well enough without fixing, but it's worth trying. Share with us the improvements and fixes if you manage to integrate typings into your project

getify commented 1 month ago

This seems to have stalled indefinitely, closing (for now).