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

Type Inference Issue with Entity.get() When Used Inside a Class Constructor #683

Open
cliu- opened this issue Feb 15, 2024 · 10 comments
Open
Assignees

Comments

@cliu-
Copy link

cliu- commented Feb 15, 2024

Describe the bug

When using the Entity.get() method from the dynamodb-toolbox package within a TypeScript class, type inference works as expected if the Entity and Table instances are created outside of a class constructor. However, moving the instantiation inside the class constructor causes TypeScript to fail to infer the correct type for the Item from the DynamoDB response, resulting in a type error.

To Reproduce

Steps to reproduce the behavior:

  • Create a Table and an Entity instance inside a TypeScript class constructor.
  • Try to use the Entity.get() method to fetch an item from DynamoDB.
  • TypeScript compiler throws a type error: Property ‘Item’ does not exist on type ‘GetCommandInput’. ts(2339).

Expected behavior

TypeScript should correctly infer the type of the Item returned by the Entity.get() method(and other methods), similar to when the Table and Entity instances are created outside the class constructor.

Code Snippets

Working scenario (Outside Constructor):

import { DynamoDBClient } from '@aws-sdk/client-dynamodb'
import { DynamoDBDocumentClient } from '@aws-sdk/lib-dynamodb'
import { Table, Entity } from 'dynamodb-toolbox'

export class DynamoService {

  marshallOptions = {
      convertEmptyValues: false, // if not false explicitly, we set it to true.
    };
  translateConfig = { marshallOptions };
  DocumentClient = DynamoDBDocumentClient.from(new DynamoDBClient({}), translateConfig);


  readonly myTable = new Table({
      name: 'my-table',
      partitionKey: 'pk',
      sortKey: 'sk',
      this.DocumentClient
    });

  readonly customer = new Entity({
      name: 'Customer',
      attributes: {
        id: { partitionKey: true }, // flag as partitionKey
        sk: { hidden: true, sortKey: true }, // flag as sortKey and mark hidden
        age: { type: 'number' }, // set the attribute type
        name: { type: 'string', map: 'data' }, // map 'name' to table attribute 'data'
        emailVerified: { type: 'boolean', required: true }, // specify attribute as required
        co: { alias: 'company' }, // alias table attribute 'co' to 'company'
        status: ['sk', 0], // composite key mapping
        date_added: ['sk', 1] // composite key mapping
      },
      table: MyTable
    } as const);

  async getItem() {
    const primaryKey = {
      id: 123,
      status: 'active',
      date_added: '2020-04-24'
    };
    const { Item } = await Customer.get(primaryKey);

    return Item;
  }
}

Non-working scenario (Inside Constructor):

import { DynamoDBClient } from '@aws-sdk/client-dynamodb'
import { DynamoDBDocumentClient } from '@aws-sdk/lib-dynamodb'
import { Table, Entity } from 'dynamodb-toolbox'

export class DynamoService {

  readOnly myTable: Table<string, string, string>;

  readOnly customer: Entity;

  constructor() {
    const marshallOptions = {
      convertEmptyValues: false, // if not false explicitly, we set it to true.
    };
    const translateConfig = { marshallOptions };
    const DocumentClient = DynamoDBDocumentClient.from(new DynamoDBClient({}), translateConfig);

    this.myTable = new Table({
      name: 'my-table',
      partitionKey: 'pk',
      sortKey: 'sk',
      DocumentClient
    });

    this.customer = new Entity({
      name: 'Customer',
      attributes: {
        id: { partitionKey: true }, // flag as partitionKey
        sk: { hidden: true, sortKey: true }, // flag as sortKey and mark hidden
        age: { type: 'number' }, // set the attribute type
        name: { type: 'string', map: 'data' }, // map 'name' to table attribute 'data'
        emailVerified: { type: 'boolean', required: true }, // specify attribute as required
        co: { alias: 'company' }, // alias table attribute 'co' to 'company'
        status: ['sk', 0], // composite key mapping
        date_added: ['sk', 1] // composite key mapping
      },
      table: MyTable
    } as const);
  }

  async getItem() {
    const primaryKey = {
      id: 123,
      status: 'active',
      date_added: '2020-04-24'
    };
    // Error: Property ‘Item’ does not exist on type ‘GetCommandInput’. ts(2339)
    const { Item } = await Customer.get(primaryKey);

    return Item;
  }
}
@anton-107
Copy link

the same happens with Entity.update - for some reason typescript thinks, that the UpdateCommandInput is returned from .update()

@nevolgograd
Copy link

Getting same error with get operation:

Property ‘Item’ does not exist on type ‘GetCommandInput’. ts(2339)

@codingnuclei
Copy link

Hey.

So this is really a typescript issue/understanding.

When you use readOnly customer: Entity; you have not defined any typings for your entity. If you want to do implicit typing you need to implicitly type your entity.

readonly customer: Entity<'Customer', any , any , any, true>;

I have missed out a lot of the generics here as there are many and the above is just an example.

The key one in your case is the last one true which says this entity is AutoExecute.

This will fix your problem but requires all generics to be properly defined to prevent other problems :)

@nevolgograd
Copy link

@codingnuclei

it didn't fix my issue unfortunately, but I have a bit more complex scenario

const jobEntity = new Entity<typeof jobEntityName, Job, CompositeKey, typeof appTable, true>({
  name: jobEntityName,
  table: appTable,
  ....

I use repository pattern:

class JobRepository extends DynamoDbRepository<typeof jobEntity> {
  constructor() {
    super(appTable, jobEntity);
  }
 ...
export abstract class DynamoDbRepository<E extends Entity> {
  constructor(
    private readonly table: Table<any, any, any>,
    private readonly entity: E,
  ) {}

  get<O extends GetOptions<E>>(primaryKey: { pk: string; sk: string }, options: O = {} as O) {
    return this.entity.get(primaryKey, options);
  }
 ...

and get operation still throws the error:

// TS2339: Property 'Item' does not exist on type 'GetCommandInput'.
const { Item } = await this.jobRepository.get({ pk: '', sk: '' });

@naorpeled
Copy link
Collaborator

naorpeled commented Mar 2, 2024

Hey everyone,
sorry for the delayed response.

In this kind of case, if necessarily you have to put the entity/table defs within the constructor, I'd recommend to not explicitly type the entity and table class variables and let TypeScript do its magic.
For the above, non-working example, when not specifying the types I get the following:
Screen Shot 2024-03-03 at 1 55 42

Otherwise, you should go for the working example's approach.

The reason for both approaches being better is that currently the Entity type definition has to have very specific arguments in order to allow your desired behavior and it's not straight forward to do that manually unfortunately.

@nevolgograd
Copy link

nevolgograd commented Mar 3, 2024

@naorpeled it is not clear what you suggest to do, can you please provide a working example?

@naorpeled
Copy link
Collaborator

naorpeled commented Apr 19, 2024

Hey @nevolgograd,
sorry for the huge delay.
I'll have another look at this tomorrow and send a better example.

@naorpeled naorpeled self-assigned this Apr 19, 2024
@naorpeled
Copy link
Collaborator

naorpeled commented Apr 20, 2024

Hey @nevolgograd,
did some more digging and honestly this doesn't seem to be as straightforward as I thought it would be.
The solution I suggested above, that seemed to work few weeks ago, now doesn't seem to do the trick.

Need to give this more thought but not 100% sure how to resolve this within v0 atm.
Sorry for my misdirection.

@nevolgograd
Copy link

@naorpeled thanks for investigating, it felt like something confusing for a reason.
Can you share insights on when v1 planning to be released?

p.s. huge thanks for this tool, I love it so much!

@tanuj-g
Copy link

tanuj-g commented May 7, 2024

@naorpeled
I am also facing the same issue, any update?

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

No branches or pull requests

6 participants