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

TS - kiota abstractions must be a peer dependency in auth and fetch library #42

Open
nikithauc opened this issue Feb 16, 2022 · 7 comments
Labels
enhancement New feature or request goodfirstissue Standard GitHub label used for easy to resolve issues targeting beginner contributors TypeScript
Milestone

Comments

@nikithauc
Copy link
Contributor

When should you use peer dependencies?
When you are building a library to be used by other projects, and
This library is using some other library, and
You expect/need the user to work with that other library as well

https://stackoverflow.com/a/34645112/10760931

@nikithauc nikithauc added enhancement New feature or request TypeScript labels Feb 16, 2022
@nikithauc nikithauc self-assigned this Feb 16, 2022
@baywet
Copy link
Member

baywet commented Feb 16, 2022

Thanks for raising this.
Assuming the abstraction lib is a peer dep for http, serialization and authentication packages, what would be the dependency story in msgraph-core? and in msgraph service libs?
Would you have them + abstractions being a peer dep in msgraph-core and then being a direct dep in the service libs to lock down the versions?

@nikithauc
Copy link
Contributor Author

Would you have them + abstractions being a peer dep in msgraph-core and then being a direct dep in the service libs to lock down the versions?

We will have abstractions as a dependency in graph core and service library.

the graph core depends on the interfaces for RequestOptions and authentication which are needed for a core only user as well.

@baywet
Copy link
Member

baywet commented Feb 18, 2022

but then following that logic, http/serialization/authentication which all depend on and expose abstractions interfaces should also have a dependency, not a peer dep.

@nikithauc nikithauc transferred this issue from microsoft/kiota Mar 25, 2022
@baywet baywet assigned koros and unassigned nikithauc Feb 24, 2023
@baywet
Copy link
Member

baywet commented Apr 26, 2023

@gavinbarron can you provide input on this one please?

@sebastienlevert
Copy link

@gavinbarron to provide input on this topic. I think the way we have it is absolutely OK and shouldn't be a peer dependency. Especially for a first-run experience that would add a lot of complexity IMO.

@baywet
Copy link
Member

baywet commented Feb 21, 2024

@gavinbarron @musale I'd appreciate your input on this one

@baywet baywet added this to the Backlog milestone Feb 21, 2024
@musale
Copy link
Contributor

musale commented Feb 23, 2024

Interesting. I would strongly agree that we put abstractions as a peer dependency if it was an ever-changing lib. I think it has a pretty finalized api that doesn't change that often. Hence having it as it is works just fine. In the event things change api wise for abstractions, then setting it as a peer dependency will offer a better experience for http/authentincation/serialization lib usage.

@fey101 fey101 added goodfirstissue Standard GitHub label used for easy to resolve issues targeting beginner contributors and removed Standard GitHub label labels Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request goodfirstissue Standard GitHub label used for easy to resolve issues targeting beginner contributors TypeScript
Projects
Status: Todo 📃
Development

No branches or pull requests

6 participants