appwrite/sdk-for-web

๐Ÿ› Bug Report: Document autocompletion for Databases.createDocument broken

emmiep opened this issue ยท 3 comments

emmiep commented

๐Ÿ‘Ÿ Reproduction steps

When using an IDE/editor supporting autocompletion with the Typescript Language Server, such as VS Code.

Calling Databases.createDocument with a type parameter:

interface ProjectModel extends Models.Document {
  name: string;
  whatever: number;
}

/* ... */

const project = await databases.createDocument<ProjectModel>(
  databaseId,
  collectionId,
  ID.unique(),
  {
    name: "my project",
  },
});

๐Ÿ‘ Expected behavior

I should see the properties I defined in the ProjectModel interface when VS Code autocompletes the code inside the curly brackets.

๐Ÿ‘Ž Actual Behavior

VS Code shows identifiers from the function scope, but not the properties defined in the ProjectModel interface:

SCR-20230929-q7u

Possible solution

I think this is caused by the data argument of Databases.createDocument having the type Omit<Document, keyof Models.Document>.

Since Models.Document contains a string index signature, the value of keyof Models.Document is probably string | number. That would remove all properties from the Document type parameter, basically turning it into an empty object type ({}).

๐ŸŽฒ Appwrite version

Different version (specify in environment)

๐Ÿ’ป Operating system

MacOS

๐Ÿงฑ Your Environment

Appwrite Cloud

Appwrite web SDK version:

13.0.0

VS Code version information:

Version: 1.82.2 (Universal)
Commit: abd2f3db4bdb28f9e95536dfa84d8479f1eb312d
Date: 2023-09-14T05:59:47.790Z (2 wks ago)
Electron: 25.8.1
ElectronBuildId: 23779380
Chromium: 114.0.5735.289
Node.js: 18.15.0
V8: 11.4.183.29-electron.0
OS: Darwin arm64 22.5.0

MacOS version:

13.4.1 (22F82)

๐Ÿ‘€ Have you spent some time to check if this issue has been raised before?

  • I checked and didn't find similar issue

๐Ÿข Have you read the Code of Conduct?

emmiep commented

I'm not sure why the Models.Document type needs the index signature, is this a thing to make sure people who use JS/don't use the type parameter don't get any warnings when accessing custom attributes from the documents?

Personally, I want the type safety of Typescript, so I would prefer if the Document base type didn't include that.
Is it possible to remove this from the Document, then maybe creating an AnyDocument type which could be the default type parameter value?

/* models.ts */

export type Document = {
  $id: string;
  $collectionId: string;
  $databaseId: string;
  $createdAt: string;
  $updatedAt: string;
  $permissions: string[];
};

export interface AnyDocument extends Document {
  [key: string]: any;
}

/* databases.ts */

async createDocument<Document extends Models.Document = Models.AnyDocument>(
  databaseId: string,
  collectionId: string,
  documentId: string,
  data: Omit<Document, keyof Models.Document>,
  permissions?: string[],
): Promise<Document> {
  /* ... */
}

That would probably solve this issue as well.

@emmiep,

I'm not sure why the Models.Document type needs the index signature, is this a thing to make sure people who use JS/don't use the type parameter don't get any warnings when accessing custom attributes from the documents?

Correct, this is to prevent errors when someone just calls createDocument() without passing in their own type.

Personally, I want the type safety of Typescript, so I would prefer if the Document base type didn't include that. Is it possible to remove this from the Document, then maybe creating an AnyDocument type which could be the default type parameter value?

/* models.ts */

export type Document = {
  $id: string;
  $collectionId: string;
  $databaseId: string;
  $createdAt: string;
  $updatedAt: string;
  $permissions: string[];
};

export interface AnyDocument extends Document {
  [key: string]: any;
}

/* databases.ts */

async createDocument<Document extends Models.Document = Models.AnyDocument>(
  databaseId: string,
  collectionId: string,
  documentId: string,
  data: Omit<Document, keyof Models.Document>,
  permissions?: string[],
): Promise<Document> {
  /* ... */
}

This is a great idea! We generate our SDKs using our sdk-generator and the template for the [key: string]: any; is here. The tough thing would be to figure out how to update the SDK generator to support what you're suggesting ๐Ÿง

Opening this for contributors to see if they can update the SDK generator to add this behavior.