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

Library fails in typescript compilation with exactOptionalPropertyTypes=true #6668

Open
3 of 4 tasks
FranzZemen opened this issue Nov 16, 2024 · 5 comments
Open
3 of 4 tasks
Assignees
Labels
bug This issue is a bug. closing-soon This issue will automatically close in 4 days unless further comments are made. guidance General information and guidance, answers to FAQs, or recommended best practices/resources. p2 This is a standard priority issue pending-release This issue will be fixed by an approved PR that hasn't been released yet. queued This issues is on the AWS team's backlog

Comments

@FranzZemen
Copy link

Checkboxes for prior research

Describe the bug

Pretty simple. Set exactOptionalPropertyTypes=true in 3.693.0 (this morning's latest NPM release) and compile. You will get transpiration errors such as:

BatchStatementRequest' is not assignable to type '{ Parameters?: any[]; }' with 'exactOptionalPropertyTypes: true

Set it to false, and there is no issue.

This causes problems for any project that is using the flag.

I have the following installed in my npm dependencies:

    "@aws-sdk/client-cloudwatch-logs": "^3.693.0",
    "@aws-sdk/client-s3": "^3.693.0",
    "@aws-sdk/credential-providers": "^3.693.0",
    "@aws-sdk/lib-dynamodb": "^3.693.0",

It does NOT happen in 3.687.0. Not sure about versions in between.

Node V20.2.0, Typescript 5.6.3 for both working and non-working versions

Regression Issue

  • Select this option if this issue appears to be a regression.

SDK version number

@aws-sdk/[email protected]

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

Node V20.2.0, Typescript 5.6.3

Reproduction Steps

set exactOptionalPropertyTypes=true and transpile using tsc

Observed Behavior

Typescript errors within AWS v3 library such as

BatchStatementRequest' is not assignable to type '{ Parameters?: any[]; }' with 'exactOptionalPropertyTypes: true

Expected Behavior

No typescript errors

Possible Solution

workaround is to set exactOptionalPropertyTypes=false

Additional Information/Context

No response

@FranzZemen FranzZemen added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 16, 2024
@zshzbh
Copy link
Contributor

zshzbh commented Nov 18, 2024

Hey @FranzZemen ,

Could you please provide a minimal code reproduction? The error occurs because with exactOptionalPropertyTypes: true, we need to be more precise with our type definitions.

Thanks!
Maggie

@zshzbh zshzbh self-assigned this Nov 18, 2024
@zshzbh zshzbh added p2 This is a standard priority issue response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Nov 18, 2024
@zshzbh
Copy link
Contributor

zshzbh commented Nov 18, 2024

I worked on this a fair bit-

The dependencies I have

  "dependencies": {
    "@aws-sdk/client-cloudwatch-logs": "^3.693.0",
    "@aws-sdk/client-dynamodb": "^3.693.0",
    "@aws-sdk/client-s3": "^3.693.0",
    "@aws-sdk/credential-providers": "^3.693.0",
    "@aws-sdk/lib-dynamodb": "^3.693.0",
    "@types/node": "^20.2.0",
    "ts-node": "^x.x.x",
    "typescript": "^5.6.3"
  }

In tsconfig.json, I set "exactOptionalPropertyTypes": true,

I ran npm run build , and I don't see any error. I can share my code below, hopefully it can be a good reference to solve the issue you have . In the code below, I use executeBatchStatements to insert the sample data and batchReadItems to read the data-

import { 
	DynamoDBClient, 
	BatchExecuteStatementCommand,
	BatchStatementRequest,
	AttributeValue
  } from "@aws-sdk/client-dynamodb";


  // Define interfaces
  interface Item {
	id: string;
	name: string;
	category?: string;
	price?: number;
  }
  
  interface BatchOperationResponse {
	success: boolean;
	data?: any;
	error?: Error;
  }
  
  // DynamoDB client configuration
  const client = new DynamoDBClient({ 
	region: "us-west-2" 
  });
  
  // Main batch operation function
  async function executeBatchStatements(items: Item[]): Promise<BatchOperationResponse> {
	const statements: BatchStatementRequest[] = items.map((item) => {
	  const parameters: AttributeValue[] = [
		{ S: item.id },
		{ S: item.name },
		item.category ? { S: item.category } : { NULL: true },
		item.price !== undefined ? { N: item.price.toString() } : { NULL: true }
	  ];
  
	  return {
		Statement: "INSERT INTO Products VALUE {'id': ?, 'name': ?, 'category': ?, 'price': ?}",
		Parameters: parameters,
		ConsistentRead: true
	  };
	});
  
	try {
	  const command = new BatchExecuteStatementCommand({
		Statements: statements
	  });
  
	  const response = await client.send(command);
  
	  if (response.Responses) {
		const errors = response.Responses.filter(r => r.Error);
		if (errors.length > 0) {
		  console.warn('Some statements failed:', errors);
		}
	  }
  
	  return {
		success: true,
		data: response.Responses
	  };
  
	} catch (error) {
	  console.error('Batch operation failed:', error);
	  return {
		success: false,
		error: error as Error
	  };
	}
  }
  
  // Batch read operation
  async function batchReadItems(ids: string[]): Promise<BatchOperationResponse> {
	const statements: BatchStatementRequest[] = ids.map(id => ({
	  Statement: "SELECT * FROM Products WHERE id = ?",
	  Parameters: [{ S: id }],
	  ConsistentRead: true
	}));
  
	try {
	  const command = new BatchExecuteStatementCommand({
		Statements: statements
	  });
  
	  const response = await client.send(command);
	  
	  return {
		success: true,
		data: response.Responses
	  };
  
	} catch (error) {
	  console.error('Batch read operation failed:', error);
	  return {
		success: false,
		error: error as Error
	  };
	}
  }
  
  
  // Example usage
  async function main() {
	const items: Item[] = [
	  {
		id: "1",
		name: "Product 1",
		category: "Electronics",
		price: 299.99
	  },
	  {
		id: "2",
		name: "Product 2",
		category: "Books",
		price: 19.99
	  }
	];
  
	console.log('Executing batch insert...');
	const insertResult = await executeBatchStatements(items);
	console.log('Insert result:', insertResult);
  
	console.log('Executing batch read...');
	const readResult = await batchReadItems(["1", "2"]);
	console.log('Read result:', readResult);
  }
  
  // Execute with error handling
  main()
	.then(() => console.log('All operations completed'))
	.catch(error => console.error('Error in main execution:', error));
  

Since I don't have the minimal code reproduction yet, I'd like to suggest to check the types, and use precise types instead of any for Optional Properties.

If you still have questions, please share minimal code reproduction and we can help you from there!

Thanks!
Maggie

@zshzbh zshzbh added the guidance General information and guidance, answers to FAQs, or recommended best practices/resources. label Nov 18, 2024
@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. label Nov 19, 2024
@FranzZemen
Copy link
Author

FranzZemen commented Nov 19, 2024

In the repo below, in tsconfig.base.json, with "exactOptionalPropertyTypes": true it produces the errors. With "exactOptionalPropertyTypes": false it does not.

Commands:

git clone https://github.com/FranzZemen/aws.3.693.0.bug
cd AWS.3.693.0.bug
npm i
./node_modules/typescript/bin/tsc

Or find relevant info below

Repo @franzzemen/aws.3.693.0.bug

If more convenient here are the need files. Test src below (can't attach .ts file)
package.json
tsconfig.base.json
tsconfig.json

source file contents, place in ./src/test.ts

import {DynamoDBDocument} from '@aws-sdk/lib-dynamodb';
import {DynamoDBClient} from '@aws-sdk/client-dynamodb';


export class DynamoClient {
  private documentClient: DynamoDBDocument;
  private dbClient: DynamoDBClient;

  constructor() {

    this.dbClient = new DynamoDBClient();
    this.documentClient = DynamoDBDocument.from(this.dbClient, {
      marshallOptions: {
        removeUndefinedValues: true
      }
    });
  }
}

@monholm
Copy link
Contributor

monholm commented Nov 19, 2024

And even simpler repro with some additional context in the readme can be found at: https://github.com/monholm/aws-sdk-ts-repro

The issue was introduced in v3.691.0 and remains in the latest version (v3.693.0 as of 2024-11-19)
Downgrading to @aws-sdk/[email protected] and @aws-sdk/[email protected] will resolve it.
These versions are the latest versions released prior to v3.691.0.

monholm added a commit to monholm/aws-sdk-js-v3 that referenced this issue Nov 19, 2024
aws#6654 added `| undefined` to optional model props, but the types in lib-dynamodb wasn't updated to
reflect that change. This leaves consumers using `exactOptionalPropertyTypes: true` with ts errors
when trying to use almost any of the commands in lib-dynamodb

Fixes aws#6668
@kuhe kuhe self-assigned this Nov 20, 2024
@kuhe kuhe added queued This issues is on the AWS team's backlog pending-release This issue will be fixed by an approved PR that hasn't been released yet. labels Nov 20, 2024
@kuhe kuhe closed this as completed in fb0e14e Nov 20, 2024
@kuhe kuhe reopened this Nov 20, 2024
@kuhe
Copy link
Contributor

kuhe commented Nov 20, 2024

merged #6683, will be released in the next published version expected tomorrow.

@kuhe kuhe added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. closing-soon This issue will automatically close in 4 days unless further comments are made. guidance General information and guidance, answers to FAQs, or recommended best practices/resources. p2 This is a standard priority issue pending-release This issue will be fixed by an approved PR that hasn't been released yet. queued This issues is on the AWS team's backlog
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants