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

feat(lib): change higher level libs use peer deps on clients #2516

Merged
merged 2 commits into from Jun 24, 2021

Conversation

alexforsyth
Copy link
Contributor

@alexforsyth alexforsyth commented Jun 23, 2021

Issue

Issue number, if available, prefixed with "#"

Description

  • Going forward: All higher order libs that allow users to "bring their own client" should declare that client as a peer dep.

Changes lib/storage and lib/dynamodb to use peer dependencies instead of pinned dependencies over their clients.

This fixes a bunch of type errors where the libs would expect the latest types without actually needing them. This caused customer pain when attempting to use packages modularly because the version of the lib would always have to be the same version as the client passed into the lib.

CC: @AllanZhengYP and I worked on this yesterday. This probably solves several issues in the backlog too.

Testing

How was this change tested?

  • Published to local verdaccio. Verified that the libs are using the version of s3 that I supplied and not the latest. No type errors.

Additional context

Add any other context about the PR here.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov-commenter
Copy link

codecov-commenter commented Jun 23, 2021

Codecov Report

❗ No coverage uploaded for pull request base (main@608e606). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2516   +/-   ##
=======================================
  Coverage        ?   59.54%           
=======================================
  Files           ?      494           
  Lines           ?    26394           
  Branches        ?     6269           
=======================================
  Hits            ?    15715           
  Misses          ?    10679           
  Partials        ?        0           

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 608e606...9d32ccb. Read the comment docs.

@AllanZhengYP
Copy link
Contributor

Is @aws-sdk/smithy-client required in peer dependency? It doesn't provide interface but only provides implementation, we want to make sure it's up-to-date if possible

@alexforsyth
Copy link
Contributor Author

I think we want @aws-sdk/smithy-client to be of the same version that @aws-sdk/client-dynamodb depends on, right?

@alexforsyth alexforsyth merged commit 2e662cd into aws:main Jun 24, 2021
@github-actions
Copy link

github-actions bot commented Jul 9, 2021

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants