Bug: Inferred type for `DynamoDBStreamRecord` (schema) is incorrect

[benthorner]

Expected Behavior

The type inferred for DynamoDBStreamRecord (schema) should include e.g. SequenceNumber.

powertools-lambda-typescript/packages/parser/src/schemas/dynamodb.ts

Line 12 in bccd0b1

SequenceNumber: z.string(),

Current Behavior

The inferred type seems to be the lowest common denominator of:

powertools-lambda-typescript/packages/parser/src/schemas/dynamodb.ts

Lines 31 to 32 in bccd0b1

	| z.infer<typeof DynamoDBStreamChangeRecordBase>
	| z.infer<typeof DynamoDBStreamToKinesisChangeRecord>,

powertools-lambda-typescript/packages/parser/src/schemas/dynamodb.ts

Lines 22 to 27 in bccd0b1

	const DynamoDBStreamToKinesisChangeRecord = DynamoDBStreamChangeRecordBase.omit(
	{
	SequenceNumber: true,
	StreamViewType: true,
	}
	);

Code snippet

import {
  DynamoDBStreamRecord as DynamoDBStreamRecordSchema,
} from '@aws-lambda-powertools/parser/schemas/dynamodb';

type DynamoDBStreamRecord = z.infer<typeof DynamoDBStreamRecordSchema>;

Steps to Reproduce

1. Write the above code snippet.
2. Hover over the DynamoDBStreamRecord type to see it does not include e.g. dynamodb.SequenceNumber.

Possible Solution

Define the dynamodb field as a union e.g.

// Make the function generic so we can force particular return types.
const unmarshallDynamoDBTransform = <T extends z.infer<typeof DynamoDBStreamChangeRecordBase> | z.infer<typeof DynamoDBStreamToKinesisChangeRecord>>(
  object: T,
  ctx: z.RefinementCtx
): T => { ... }

// Specialise `DynamoDBStreamChangeRecord` to the full / base type.
const DynamoDBStreamChangeRecord = DynamoDBStreamChangeRecordBase.transform(
  unmarshallDynamoDBTransform<z.infer<typeof DynamoDBStreamChangeRecordBase>>
);

// Add a further type to accommodate the "ToKinesis" version.
const DynamoDBStreamToKinesisChangeRecord = DynamoDBStreamChangeRecordBase.transform(
  unmarshallDynamoDBTransform<z.infer<typeof DynamoDBStreamToKinesisChangeRecord>>
);

// Define `dynamodb field as a union.
const DynamoDBStreamRecord = z.object({
  ...,
  dynamodb: z.union([DynamoDBStreamChangeRecord, DynamoDBStreamToKinesisChangeRecord]),
  ...,
});

Powertools for AWS Lambda (TypeScript) version

latest

AWS Lambda function runtime

22.x

Packaging format used

npm

Execution logs

[dreamorosi]

Hi @benthorner, thank you for opening this issue.

The team is in the middle of a release, we'll take a look at it on Monday morning at the latest.

[svozza]

Yes, if SequenceNumber is missing then this is a bug imo. Thank you for such a detailed report btw. Would you be interested in raising a PR for this?

[benthorner]

@svozza thanks for taking a look!

I could try for a PR. Before I do, I have a few questions (to save time):

1. Do you have any thoughts on my proposed approach? If it looks undesirable then better to know now.
2. What do you think about consistently suffixing everything in schemas/dynamodb.ts with Schema?

The reason for (2) is that, ideally, we would be able to do this in schemas.js:

// before
const DynamoDBStreamChangeRecord[Base] = DynamoDBStreamChangeRecordBase.transform(
  unmarshallDynamoDBTransform<z.infer<typeof DynamoDBStreamChangeRecordBase>>
);

// after
import { DynamoDBStreamChangeRecordBase } from "../types/schema.js"

const DynamoDBStreamChangeRecord[Base]Schema = DynamoDBStreamChangeRecordBase.transform(
  unmarshallDynamoDBTransform<DynamoDBStreamChangeRecordBase>
);

Otherwise we have to juggle two variables with the same name - one of which is the schema.

This would be a bit of a breaking change if anyone is importing those properties already - with their current names.

[svozza]

1. Your approach seems reasonable to me so I don't have concerns there.
2. Unfortunately, we can't do breaking changes with a major version bump, which we don't have plans for currently so we'll need to keep the names as they are. However, I do agree with you in principle so this could definitely be something we could change in v3.

[dreamorosi]

We have an ongoing discussion for v3 features, if you land on a naming convention you're not happy with please add it to the discussion as a comment, so the when we start working on v3 we can remember about it.

[benthorner]

@svozza in the meantime, one option would be to do this:

export {
  DynamoDBStreamChangeRecord[Schema] as DynamoDBStreamChangeRecord,
}

This keeps preserves the external API but avoids name clashes within the module, unblocking defining more types.

Does that sound OK to you? Again, just asking to avoid wasting time.