typesafe-dynamodb icon indicating copy to clipboard operation
typesafe-dynamodb copied to clipboard

Incorrect type inference from GetItem command (v3)

Open volkanunsal opened this issue 2 years ago • 14 comments

The type of Item appears to be undefined. I expected it to be IAccount

Screen Shot 2022-06-19 at 5 40 19 PM

Here is the code

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

const client1 = new DynamoDBClient({});

export const docClient = DynamoDBDocumentClient.from(
  client1
) as TypeSafeDocumentClientV3<IAccount, 'pk', 'sk'>;

const res1 = await docClient.send(
  new TypedGetItemCommand({
    TableName: 'Account',
    Key: {
      pk: 'ACCOUNT#foo',
      sk: 'ACCOUNT#foo',
    },
  })
);
res1.Item;

volkanunsal avatar Jun 19 '22 21:06 volkanunsal

How is TypedGetItemCommand defined?

sam-goodwin avatar Jun 19 '22 21:06 sam-goodwin

export const TypedGetItemCommand = TypeSafeGetDocumentCommand<
  IAccount,
  'pk',
  'sk'
>();

volkanunsal avatar Jun 19 '22 21:06 volkanunsal

Hmm, I can't seem to reproduce this error.

image

sam-goodwin avatar Jun 20 '22 23:06 sam-goodwin

Tried the same code in a separate package to make sure it works when importing typesafe-dynamodb from node_modules and it works.

import { DynamoDBClient } from "@aws-sdk/client-dynamodb";
import { DynamoDBDocumentClient } from "@aws-sdk/lib-dynamodb";
import { TypeSafeDocumentClientV3 } from "typesafe-dynamodb/lib/document-client-v3";
import { TypeSafeGetDocumentCommand } from "typesafe-dynamodb/lib/get-document-command";

const client1 = new DynamoDBClient({});

export interface IAccount {
  pk: string;
  sk: string;
}

export const docClient = DynamoDBDocumentClient.from(
  client1
) as TypeSafeDocumentClientV3<IAccount, "pk", "sk">;

export const TypedGetItemCommand = TypeSafeGetDocumentCommand<
  IAccount,
  "pk",
  "sk"
>();

async function main() {
  const res1 = await docClient.send(
    new TypedGetItemCommand({
      TableName: "Account",
      Key: {
        pk: "ACCOUNT#foo",
        sk: "ACCOUNT#foo",
      },
    })
  );
  res1.Item;
}

sam-goodwin avatar Jun 20 '22 23:06 sam-goodwin

The problem is in the generics. Try with this type:

export interface IAccount<
  id extends string = string,
  key extends string = string
> {
  pk: `ACCOUNT${id}`;
  sk: `ACCOUNT${key}`;
}

volkanunsal avatar Jun 21 '22 00:06 volkanunsal

Got it, now I can repro. Will dig in.

sam-goodwin avatar Jun 21 '22 00:06 sam-goodwin

Hmm, it works when the keys are parameterized but not when hard-coded.

export interface IAccount<
  id extends string = string,
  key extends string = string
> {
  pk: `ACCOUNT#${id}`;
  sk: `ACCOUNT#${key}`;
}

const GetUnionItemCommand = TypeSafeGetDocumentCommand<IAccount, "pk", "sk">();

export async function union(pk: string, sk: string) {
  const item = await client.send(
    new GetUnionItemCommand({
      TableName: "foo",
      Key: {
        pk: `ACCOUNT#${pk}`,
        sk: `ACCOUNT#${sk}`,
      },
    })
  );

  item.Item?.pk;
}

sam-goodwin avatar Jun 21 '22 01:06 sam-goodwin

Narrowed down the problem to Narrow.

// is `never` because TS isn't smart enough to detect the overlap
type A = Extract<
  IAccount,
  {
    pk: "ACCOUNT#pk";
    sk: `ACCOUNT#${string}`;
  }
>

This may be a limitation of string literal types in typescript.

image

The reason for why Item is undefined in your example is because never | undefined reduces to undefined.

sam-goodwin avatar Jun 21 '22 01:06 sam-goodwin

The workaround is to not use hard-coded strings.

const pk: string = "pk";
const sk: string = "sk";
const item = await client.send(
  new GetUnionItemCommand({
    TableName: "foo",
    Key: {
      pk: `ACCOUNT#${pk}`,
      sk: `ACCOUNT#${sk}`,
    },
  })
);

Note how I store the strings as const pk: string and especially note how i explicitly type it as : string. This is so that TypeScript will properly infer the type as ACCOUNT#${string}.

I'll keep digging to see if there is something we can to workaround this in typesafe-dynamdob.

sam-goodwin avatar Jun 21 '22 01:06 sam-goodwin

I don't think we can workaround this issue. It is not an expected use-case to have hard-coded suffixes in the keys that aren't properly represented in the type (IAccount). It will only work if the types are parameterized.

You can see the general problem here - ACCOUNT#${string} is not assignable to ACCOUNT#abc and therefore narrows to never. Maybe TS could be smarter here and realize that ACCOUNT#${string} should be narrowed to the literal string ACCOUNT#abc. I will cut an issue with the TypeScript team and see what they think.

image

sam-goodwin avatar Jun 21 '22 01:06 sam-goodwin

Yeah, that is a bummer.

volkanunsal avatar Jun 21 '22 01:06 volkanunsal

We will see what the TypeScript team has to say about this https://github.com/microsoft/TypeScript/issues/49617

sam-goodwin avatar Jun 21 '22 01:06 sam-goodwin

Need to try intersection (&) instead of Extract

sam-goodwin avatar Jun 21 '22 03:06 sam-goodwin

Intersection types do seem to behave better here.

image

sam-goodwin avatar Jun 21 '22 03:06 sam-goodwin

This issue is now marked as stale because it hasn't seen activity for a while. Add a comment or it will be closed soon. If you wish to exclude this issue from being marked as stale, add the "backlog" label.

github-actions[bot] avatar Aug 21 '22 03:08 github-actions[bot]

Closing this issue as it hasn't seen activity for a while. Please add a comment @mentioning a maintainer to reopen. If you wish to exclude this issue from being marked as stale, add the "backlog" label.

github-actions[bot] avatar Aug 28 '22 03:08 github-actions[bot]