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

refactor: strict types #2802

Merged
merged 15 commits into from
Nov 4, 2021
Merged

refactor: strict types #2802

merged 15 commits into from
Nov 4, 2021

Conversation

Thisen
Copy link
Contributor

@Thisen Thisen commented Sep 20, 2021

Reasoning 💡

Comply with TS strict typing. Very much WIP.

Checklist 🧢

  • Documentation - N/A
  • Tests - N/A
  • Ready to be merged - N/A

Affected issues 🎟

Partially fixes #2709. TypeScript conversion should be done in a new PR, to keep this PR more concise.

@github-actions github-actions bot added core Refers to `@auth/core` pages providers labels Sep 20, 2021
@Thisen
Copy link
Contributor Author

Thisen commented Oct 1, 2021

@balazsorban44, I've got good progress, however there are two outstanding issues, where I'm unsure what to do:

  • in client-legacy.ts, where some get methods from oauth1Client are being promisfied. I think the intent was to replace the callback, but that's not what happening.
  • in src/providers/oauth.ts, this line (export type { OAuthProviderType } from "./oauth-types"), but oauth-types doesn't exist.

@balazsorban44
Copy link
Member

balazsorban44 commented Oct 1, 2021

  1. oauth1Client has a callback-based API, while we want to use a Promise-based one. I don't think there is a graceful way of typing that part, I would just set it anywhere necessary. 🤷
  2. that is generated by npm run generate-providers

@codecov-commenter
Copy link

codecov-commenter commented Oct 1, 2021

Codecov Report

Merging #2802 (72be9a8) into beta (4181988) will increase coverage by 0.11%.
The diff coverage is 15.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##             beta    #2802      +/-   ##
==========================================
+ Coverage   13.76%   13.87%   +0.11%     
==========================================
  Files          86       86              
  Lines        1359     1362       +3     
  Branches      375      376       +1     
==========================================
+ Hits          187      189       +2     
- Misses       1163     1164       +1     
  Partials        9        9              
Impacted Files Coverage Δ
src/core/errors.ts 0.00% <0.00%> (ø)
src/core/index.ts 0.00% <0.00%> (ø)
src/core/lib/cookie.ts 0.00% <0.00%> (ø)
src/core/lib/email/signin.ts 0.00% <ø> (ø)
src/core/lib/oauth/authorization-url.ts 0.00% <0.00%> (ø)
src/core/lib/oauth/callback.ts 0.00% <0.00%> (ø)
src/core/lib/oauth/client-legacy.ts 0.00% <0.00%> (ø)
src/core/lib/oauth/client.ts 0.00% <ø> (ø)
src/core/lib/oauth/pkce-handler.ts 0.00% <0.00%> (ø)
src/core/lib/providers.ts 0.00% <0.00%> (ø)
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4181988...72be9a8. Read the comment docs.

@Thisen Thisen marked this pull request as ready for review October 1, 2021 13:21
@Thisen
Copy link
Contributor Author

Thisen commented Oct 4, 2021

@balazsorban44, feel free to review. :)

@balazsorban44
Copy link
Member

balazsorban44 commented Oct 28, 2021

@Thisen could you fix the merge conflicts? I changed a bunch of stuff through #2857 and that PR actually converted the last core files to TS as well, only some providers remain js, feel free to convert them as well

@Thisen
Copy link
Contributor Author

Thisen commented Oct 28, 2021

I will 👍

@github-actions github-actions bot added client Client related code and removed pages labels Oct 31, 2021
@Thisen
Copy link
Contributor Author

Thisen commented Oct 31, 2021

@balazsorban44, it's done. I think the converting the providers to TS should be a different PR, this one is already rather large.

@Thisen
Copy link
Contributor Author

Thisen commented Nov 4, 2021

@balazsorban44, since you asked for me to fix the conflicts, when will this be merged?

@balazsorban44 balazsorban44 changed the title Strict types refactor: strict types Nov 4, 2021
@balazsorban44 balazsorban44 merged commit f998bf2 into nextauthjs:beta Nov 4, 2021
mnphpexpert added a commit to mnphpexpert/next-auth that referenced this pull request Sep 2, 2024
* WIP strict types

* wip types

* wip strict types

* More strict typing

* Removing strict false
Fix last types

* Fix typo

* Make TS happy

* Fix tests

* Fixes to types

* Make files align with strict mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Client related code core Refers to `@auth/core` providers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants