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

suggestion: Move OAuth2ClientConfig interface into src/types.ts #38

Open
jollytoad opened this issue Sep 14, 2023 · 3 comments
Open

suggestion: Move OAuth2ClientConfig interface into src/types.ts #38

jollytoad opened this issue Sep 14, 2023 · 3 comments

Comments

@jollytoad
Copy link

To allow a consuming project to utilitize the config type without pulling in the full dependency of the OAuth2Client.

This could be completely backwards compatible by re-exporting the type from the oauth2_client.ts

Happy to submit PR if you are open to this.

@iuioiua
Copy link
Contributor

iuioiua commented Sep 17, 2023

This is a server-side module. A server downloading the whole module of 33 KB is negligible. This would be a different story if this were a client-side module where dependency sizes have a tangible effect. While this would be nice to have, this is not something to be concerned about.

@jollytoad
Copy link
Author

jollytoad commented Sep 18, 2023

They all add up, and it's not so much about the size but the complexity of dependencies. Deno provides a very simple way to keep a dependency tree as clean as possible, by importing only exactly what you absolutely require, we should embrace it, it's the main advantage over the npm dependency hell.

If want to write functions that reference the OAuth2ClientConfig type, then I want just that, and not the whole implementation, and there is no reason I shouldn't be able to do that if types are just exported separated, it's a pretty easy thing for a library to do, and doesn't break compatibility.

@cmd-johnson
Copy link
Owner

Sounds reasonable

I'll add this change to the PR with the OIDC implementation (#39), since it already includes a refactoring of the OAuth2Client class.

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