-
Notifications
You must be signed in to change notification settings - Fork 27
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(aws-sdk-v3): remove v2 types #422
Conversation
@nduthoit Wow, Thanks a lot for that contribution. This looks great and happy to hear that it worked by using the |
src/helper/fetch-all.function.ts
Outdated
@@ -11,7 +11,7 @@ import { ScanRequest } from '../dynamo/request/scan/scan.request' | |||
* available. This can be used with scan and query requests. | |||
*/ | |||
|
|||
export function fetchAll<T>(request: ScanRequest<T> | QueryRequest<T>, startKey?: DynamoDBv2.Key): Promise<T[]> { | |||
export function fetchAll<T>(request: ScanRequest<T> | QueryRequest<T>, startKey?: Record<string, AttributeValue>): Promise<T[]> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's try to stick with DynamoDB.AttributeValue to be consistent (import * as DynamoDB from '@aws-sdk/client-dynamodb'
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also updated the KeyType
imports to be consistent.
src/mapper/mapper.spec.ts
Outdated
@@ -713,13 +713,13 @@ describe('Mapper', () => { | |||
describe('model with non string/number/binary keys', () => { | |||
it('should accept date as HASH or RANGE key', () => { | |||
const now = new Date() | |||
const toDbVal: DynamoDBv2.AttributeMap = toDb(new ModelWithDateAsHashKey(now), ModelWithDateAsHashKey) | |||
const toDbVal: Record<string, AttributeValue> = toDb(new ModelWithDateAsHashKey(now), ModelWithDateAsHashKey) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here (DynamoDB.AttributeValue
)
src/mapper/mapper.spec.ts
Outdated
expect(toDbVal.startDate.S).toBeDefined() | ||
expect(toDbVal.startDate.S).toEqual(now.toISOString()) | ||
}) | ||
it('should accept date as HASH or RANGE key on GSI', () => { | ||
const now = new Date() | ||
const toDbVal: DynamoDBv2.AttributeMap = toDb( | ||
const toDbVal: Record<string, AttributeValue> = toDb( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here (DynamoDB.AttributeValue
)
a511096
to
ef63239
Compare
@michaelwittwer yes, I hear you, "works on my machine" is not necessarily the best proof 😅 I tried replicating the reported issue with KeyType using @aws-sdk/client-dynamodb@v3.188.0 but was not able to. |
@nduthoit Thanks for the update. Do you use any bundler or plain |
🎉 This PR is included in version 8.0.0-next.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
@nduthoit I just tried to compile our Angular application using
I see the issue going away starting with version |
We use webpack Nice find on the version that includes the |
Addresses this TODO by using the built-in aws-sdk v3 types and removing the need for src/aws-sdk-v2.types.ts.
BatchGetRequestMap
{ [key: string]: KeysAndAttributes }
Record<string, KeysAndAttributes>
BatchWriteItemRequestMap
{ [key: string]: WriteRequest[] }
Record<string, WriteRequest[]>
ExpressionAttributeNameMap
{ [key: string]: string }
Record<string, string>
ExpressionAttributeValueMap
{ [key: string]: AttributeValue }
Record<string, AttributeValue>
AttributeMap
{ [key: string]: AttributeValue }
Record<string, AttributeValue>
Key
{ [key: string]: AttributeValue }
Record<string, AttributeValue>
Also switches to using
KeyType
from @aws-sdk/client-dynamodb. Tested and it worked at runtime.