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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typescript typings file :) #30

Open
noomorph opened this issue Nov 7, 2022 · 6 comments
Open

Typescript typings file :) #30

noomorph opened this issue Nov 7, 2022 · 6 comments

Comments

@noomorph
Copy link

noomorph commented Nov 7, 2022

Hi, is there any plan to have one? 馃憢

@getify
Copy link
Owner

getify commented Nov 7, 2022

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
Copy link

@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
Copy link
Owner

getify commented Apr 25, 2023

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:

CAF/src/caf.js

Line 73 in a2b12de

let doCancelTimer = function cancelTimer(v){

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
Copy link
Author

noomorph commented Apr 25, 2023

Oh, thanks for reminding me about the existence of this issue 馃槅

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
Copy link

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:

CAF/src/caf.js

Line 73 in a2b12de

let doCancelTimer = function cancelTimer(v){

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
Copy link
Author

@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

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

3 participants