-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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 support #220
Typescript support #220
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/iaincollins/next-auth-docs/ah4xm0hva |
Thanks @lluia, I really appreciated you picking up the work from @falleco for v1! I left the old PR both as reference for myself, and hoping someone might be able to help with it. I just wanted to say it will probably be a few days before I can pick this up again, but it's really appreciated and it is something I would like to have. One thing it would be really nice to have some developer tests or a script that we could run with an We already have linting with eslint, using StandardJS ( |
Looking forward to this |
@iaincollins after some investigation I decided to go with It seems is what most libraries are using 🧐 ( given is my first try doing this happy to move to other suggestions... ) Currently, it can't compile due to an error on
I'm looking on how to bypass that 🤔 🤞🏻 |
Thanks! This seems like a good direction! This is not an area I'm familiar with myself, but if we can get one or two examples working I'd be very happy to pitch in and try and improve (and maintain) coverage. |
@iaincollins I opened an issue on I was thinking instead from writing the declaration files manually 🤔 , we could move the library to TS and let the compiler generate the types for us ( might be more scalable ). It'll involve though some work though 😆 |
I tried to start adding types to for the |
@arnodel I think the trick is to declare the module in this way: declare module 'next-auth/client' {
// type declarations here
} in this way, it won't matter where declaration files are located ( as per TS docs ) and hence we can make it live inside of I added an example of it working 👍🏻 |
^ Thank you to everyone working on this, I'm not commenting because I don't have anything to add to the convo but this is very much appreciated. 🙂 |
Not into TypeScript @iaincollins? |
@LoriKarikari I prefer strongly typed languages in general - C# is my all time favourite language - but I'm not personally a fan of TypeScript in practice, as I've run into too many issues with it (e.g. edge cases, compatibility issues, etc). If pushed I would say I prefer Flow, however, I get that TypeScript is the most popular way of using types in JavaScript and a lot of folks get value out of it and I'm happy to support it. I'm looking forward to seeing Deno evolve though! I think having better out of the box for types (fast and native) will make it a very different developer experience. |
Thanks @lluia for the example. However it doesn't appear to work for me with the new change - I cannot import from Previously those imports were resolved (with |
@lluia I am importing from a project that uses
That doesn't seem a good long term solution to me however (no other package that I depend on requires that). Also, when running
The link says that So perhaps it would be better to rewrite the contents of |
@arnodel good catch, I think as you suggest using To clarify: for now, the If you need them straight away for a project you're working I'd suggest the best is to use |
@lluia personally I wouldn't mind having the type definition files in the I also think it would be good to publish those types as soon as possible. IMHO as this is a fork / PR, there is no need to worry about exposing a non-stable interface to unsuspecting users, but it allows people to use it easily (knowing the risks). In my opinion lack of typescript definitions is likely to put off a fair number of potential users, so providing something usable early would be very valuable. |
@lluia what's happening with the typings now? I see that the CI is failing on DefinitelyTyped... is there anything I can do help out? |
@samsamson33 I need to have a look, hopefully, this week 👀 🤞🏻 , I think the compiler is trying to parse the types from
A workaround, for the time being, is to not relly on |
@lluia could not finding |
@samsamson33 ah good point 💥 , let me push it to the PR and see if it makes the error go away 🤞🏻 |
@iaincollins @lluia @samsamson33 @bamorim and others. awesome work on this! (I was disappointed that my original work on typings weren't updated for this release, so I came looking) I spent an hour or two trying out converting next auth to typescript. I just have to finish typing the options and a few other small issues (and taking some types from this PR). @iaincollins would you be more open to accepting a PR like this? Most of it is utilizing type inference or |
@iaincollins @lluia @samsamson33 @bamorim See the PR I just submitted for a working migration to typescript. I'm sure there's more work to be done, so please check it out and provide feedback! We might end up abandoning it, because of reasons explained earlier in this chain, but I thought I'd remove the "too much work to transition" hurdle. (because really its not hard... we could have stopped at just a rename and add a bunch of |
I favour @lluia approach in this PR, which is incremental and gives a chance to test things out as a team. With regard to TypeScript, supporting pull request is my priority right now. If anyone is looking for a way to help out to support a move to TypeScript (beyond helping out with this PR) the most useful thing right now would be to help out with the end to end tests, which have sadly been on hold for too long. |
Update 🗓So the types are finally ready on DefinitivelyTyped 💆🏻♂️ 🎊 🤞🏻 💥 I'll need now a couple of reviews (maybe @iaincollins and @NickBolles ?) to be able to merge it 💚 I'm aware they're less than a perfect but I think is a good start and we can get them out and iterate later. I think the key for them to grow strong is to sync well whenever changes happen in this library. As a first step making Why it took so long ⏲I was caught forever with a cryptic error related to The fact also that I lost my job due to COVID and been actively looking for a new one since a few weeks made a bit challenging keeping up with open-source 😅 |
@m5r thanks for your comments 💯 , I'll try to get them addressed soon. Could you next time comment on the DefinitivelyTyped PR? The work is carried on there and this one is just open for reference. |
Finally |
Great work @lluia! |
@lluia Thank you and congratulations! <3 What are your thoughts on merging this PR into NextAuth.js also, so we can work towards better support for types natively? |
are there any plans to add types for |
@iaincollins I think the types on this PR are a bit outdated, so in that case would be best to directly copy what we have on DefinitivelyTyped 🤔 I'd say for now lets see how |
I could do an exact copy of the example repo but in TypeScript. Just create an empty repo in the organization and I'll PR it |
@JuanM04 here you go if you have time 👍🏽 , would be cool to keep the file structure same as the default example repo. In case you don't find time don't worry! I'll try to PR on it once I'm back from hols ⛱ next week ( @iaincollins ) |
@lluia Thanks for this contribution, and more importantly, enjoy your holidays! 😃 |
Thank you, @lluia! Enjoy your holidays! |
I finished the repo, but some types are wrong. I opened a PR fixing them |
@lluia Thanks for your feedback! Sounds good to me. Have a great holiday! :-) |
Thank you. Should be added in the documentation as well :) with the command |
great !! thx |
Summary
Following the effort @falleco started on #71, this PR aims to bring updated types for v2.
🗺 Roadmap
Server
Server/Providers
Server/Adapters
Client
🚨 Disclaimer
I've been working with TS for a while, but this is my first time typing a library so feel free to comment extensively on what can be improved or might be incorrect. 🙏🏻
I'm also not familiar with
typeorm
or the inner workings of bothadapters
andprovider
so @iaincollins your guiding light 💡 here much appreciated 🤞🏻