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: Storage interface types #10696

Merged
merged 2 commits into from Nov 22, 2022
Merged

fix: Storage interface types #10696

merged 2 commits into from Nov 22, 2022

Conversation

stocaaro
Copy link
Contributor

Description of changes

Adding Storage generics type clarification.

Without this fix. TS Strict is expressing issues in VSCode and Angular 15 fails to build.

Issue #, if available

#10687

Description of how you validated changes

Running tests and linking this change to an existing project.

Checklist

  • PR description included
  • yarn test passes

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@stocaaro stocaaro requested a review from a team as a code owner November 21, 2022 18:26
@codecov-commenter
Copy link

Codecov Report

Merging #10696 (7221aac) into main (92cef8a) will increase coverage by 1.37%.
The diff coverage is 83.87%.

@@            Coverage Diff             @@
##             main   #10696      +/-   ##
==========================================
+ Coverage   84.28%   85.65%   +1.37%     
==========================================
  Files         256      196      -60     
  Lines       18527    18261     -266     
  Branches     3981     3892      -89     
==========================================
+ Hits        15615    15642      +27     
+ Misses       2822     2543     -279     
+ Partials       90       76      -14     
Impacted Files Coverage Δ
...ages/amazon-cognito-identity-js/src/CognitoUser.js 79.18% <64.28%> (-0.20%) ⬇️
...on-cognito-identity-js/src/AuthenticationHelper.js 100.00% <100.00%> (ø)
.../amazon-cognito-identity-js/src/CognitoUserPool.js 81.35% <100.00%> (+0.65%) ⬆️
...astore/src/storage/adapter/AsyncStorageDatabase.ts 83.92% <0.00%> (-10.52%) ⬇️
packages/datastore/src/storage/storage.ts 90.24% <0.00%> (-7.41%) ⬇️
packages/analytics/src/Analytics.ts 55.89% <0.00%> (-7.24%) ⬇️
packages/datastore/__tests__/helpers.ts 84.13% <0.00%> (-5.87%) ⬇️
.../datastore/src/storage/adapter/IndexedDBAdapter.ts 85.26% <0.00%> (-2.89%) ⬇️
packages/core/src/JS.ts 90.26% <0.00%> (-1.87%) ⬇️
packages/datastore/src/predicates/sort.ts 89.18% <0.00%> (-1.72%) ⬇️
... and 215 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@israx israx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me.

T
>;

export type StorageCopyConfig<T extends Record<string, any>> =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the Record<string, any> type being repeated on most types above. A suggestion I'm thinking would be creating a default config type

type DefaultStorageConfig = Record<string, any>

Copy link
Contributor

@elorzafe elorzafe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Thanks @stocaaro 🚢

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.

None yet

4 participants