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

Provide typing #334

Closed
Sytten opened this issue Jun 16, 2020 · 16 comments
Closed

Provide typing #334

Sytten opened this issue Jun 16, 2020 · 16 comments
Labels

Comments

@Sytten
Copy link

Sytten commented Jun 16, 2020

Context

  • node version: All
  • module version: >2.5.0
  • environment: All

What problem are you trying to solve?

The only typing available for this library in @types/simple-oauth2 is very old and is not corresponding with the latest version of the library. This basically makes the library unusable with Typescript. Would it possible to provide typing?

Thanks!

@jonathansamines
Copy link
Collaborator

@Sytten Yes, not against it, but since I don't use typescript myself I don't have the necessary knowledge to put that in place. I am open to PR that can guarantee that:

  • Providing typings work for the current major library version
  • Provide a way to ensure we don't break those types as we make changes to the library

@Sytten
Copy link
Author

Sytten commented Jun 16, 2020

@Zageron
Copy link

Zageron commented Jul 4, 2020

npm i [email protected] is a poor solution, but worked for me in the meantime. The examples in the https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/simple-oauth2/simple-oauth2-tests.ts file helped me do what I needed.

@nazarkulyk
Copy link

Typings are not compatible to changes with current version starting with 4.0.0. Last compatible version ist @types/simple-oauth2 = 2.5.3 with simple-oauth2 = 3.4.0

@namdien177
Copy link

namdien177 commented Jul 9, 2020

Having an official Typing support will be great as TypeScript is more and more popular.
And because of mismatching version of @types/simple-oauth2 and simple-oauth2, I lost the entire day to debug and fix.

Since I tired of waiting, I made a simple typing file for this library (based on the original @types/simple-oauth2). Feel free to use and comment as this is the first time I do this typing thing.

https://gist.github.com/namdien177/064db5d07850ab28714b6e41858a7e0c

@Sytten
Copy link
Author

Sytten commented Jul 9, 2020

@namdien177 I suggest you make a PR on the @types package. That would really benefit the community.

@namdien177
Copy link

Honestly, I want to do that but I don't have any experience with creating PR in the community repo; also, I still need to check and tweak my typing version for a few days to ensure it will work.
I will try to make a PR when everything is good to go.

@jimmykane
Copy link

In regards to this, when upgrading to 4.x I get in my firebase project

  at Object.<anonymous> (/Users/dimtrioskanellopoulos/Projects/track-tools/functions/node_modules/simple-oauth2/index.js:3:16)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
⚠  We were unable to load your functions code. (see above)
   - It appears your code is written in Typescript, which must be compiled before emulation.
   - You may be able to run "npm run build" in your functions directory to resolve this.

I suppose it's related.

Any tip. ?

@jimmykane
Copy link

@namdien177 how can one load/refer to your typing file ?

@namdien177
Copy link

namdien177 commented Jul 16, 2020

@namdien177 how can one load/refer to your typing file ?

I believe you can follow these steps or these?

For my current project, I just replace the content of node_modules/@types/simple-oauth2 with mine.

@jonathansamines
Copy link
Collaborator

jonathansamines commented Jul 17, 2020

@jimmykane Please also note that we do rely on some Node 12 only features on v4. Please verify your environment supports that.

@Zageron
Copy link

Zageron commented Jul 18, 2020

Oooh, thank you @namdien177 ! :D

@jimmykane
Copy link

@jonathansamines ok thank you very much. This is useful since Firebase has 10.

I ll hold back a bit and thanks again

@ziv
Copy link

ziv commented Jul 20, 2020

#342

@namdien177
Copy link

image
It's on guys.

@jonathansamines
Copy link
Collaborator

That's good news @namdien177. Since the issue seem to be solved now, will close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants