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

fix!: remove @aws-sdk types and client-s3 from peer dependencies #57

Merged
merged 8 commits into from
Jun 13, 2022

Conversation

m-radzikowski
Copy link
Owner

@m-radzikowski m-radzikowski commented Sep 29, 2021

NPM 7 installs peer dependencies automatically. This can cause installing newer version of the @aws-sdk/types than is required by other client libraries, and cause type conflicts.

We remove @aws-sdk/types and @aws-sdk/client-s3 from peer dependencies.

@aws-sdk/types package is required anyway by the SDK client libs.

The client-s3 lib is needed for libStorage mock functionality. Now, the libStorage is not imported in the main package index.ts, so lack of client-s3 will not cause errors for users not needing it. The mockLibStorageUpload must be now imported as:

import {mockLibStorageUpload} from 'aws-sdk-client-mock/libStorage';

TODO:

  • wait for jest v28 release with Support package exports in jest-resolve jestjs/jest#9771 solved - until then, jest does not support package exports, and import ... from 'aws-sdk-client-mock/libStorage' fails
  • update README for libStorage mocking, including required jest version >= 28

Solves #56

NPM 7 installs peer dependencies automatically.
This can cause installing newer version
of the @aws-sdk/types than is required by other client libraries,
and cause type conflicts.
types package is required anyway by the SDK client libs.
This will make the default package import not require @aws-sdk/client-s3 installed.
@m-radzikowski m-radzikowski marked this pull request as draft November 6, 2021 19:04
@codecov-commenter
Copy link

codecov-commenter commented Nov 6, 2021

Codecov Report

Merging #57 (7935fd6) into main (dfcc9fa) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main       #57   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          116       115    -1     
  Branches        12        12           
=========================================
- Hits           116       115    -1     
Impacted Files Coverage Δ
src/index.ts 100.00% <ø> (ø)
src/libStorage.ts 100.00% <100.00%> (ø)

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 dfcc9fa...7935fd6. Read the comment docs.

@m-radzikowski m-radzikowski changed the title fix: remove @aws-sdk types and client-s3 from peer dependencies fix!: remove @aws-sdk types and client-s3 from peer dependencies Nov 6, 2021
@sekhavati
Copy link

Looking forward to seeing this go in 👍 We were trying to upgrade all our runtimes to Node v16 given the recent AWS announcement and started to encounter these issues

@m-radzikowski m-radzikowski marked this pull request as ready for review June 13, 2022 13:49
@m-radzikowski
Copy link
Owner Author

@sekhavati thanks for the ping 🙂 It's definitely a time to merge it.

I will merge it now and release a new version later today.

@m-radzikowski m-radzikowski merged commit c66e050 into main Jun 13, 2022
@m-radzikowski m-radzikowski deleted the issue56 branch June 13, 2022 14:40
@m-radzikowski
Copy link
Owner Author

@sekhavati check out the freshly released v1.0.0 🙂

@sekhavati
Copy link

sekhavati commented Jun 16, 2022

@m-radzikowski I've given this a fresh run today and unfortunately I'm still encountering similar issues regarding type conflicts as mentioned here.

To give you some background:

  • We were on v0.6.2 of this library and Node v14 for a long time without any issues
  • AWS Lambda released support for Node v16 so we attempted to upgrade our monorepo
  • Upon doing so we immediately starting hitting the conflicting types error list above
  • We're now on Jest v28.1.1 and v1.0.0 of this library but still get the same errors
  • Each monorepo package declares its own @aws-sdk/* dependencies as needed (ie: not declared at the top level package.json), but across all packages they're on the same version (v3.49.0 for us)
  • I've tried installing @aws-sdk/types at the root level and/or at individual package level but to no avail

I'll have to come back and revisit this again antoher time now, but rather short on ideas at the moment, other than perhaps try upgrading all the @aws-sdk/* packages across the board to the latest versions and seeing if that makes a difference.

If you have any other suggestions please let me know!

Thanks again for the effort that's gone into this library, it's awesome 👍

@sekhavati
Copy link

@m-radzikowski on a whim I thought I'd try regenerating all the package-lock.json files throughout the monorepo (given we pin our dependencies anyway) and low and behold everything now works 🙌 Not sure what was up with them but thought you might like to know!

ps. above was without installing @aws-sdk/types explicitly either

@m-radzikowski
Copy link
Owner Author

@sekhavati great, I'm glad you solved it! Seems like npm did not clear no longer needed peer dependencies upon library version bump? Weird.

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

Successfully merging this pull request may close these issues.

3 participants