🐛 Bug Report: Document autocompletion for Databases.createDocument broken

[emmiep]

👟 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:

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?

• I have read the Code of Conduct

[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?

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.

[stnguyen90]

@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 🧐

[stnguyen90]

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

[ChiragAgg5k]

in our sdk generator we can shift the logic of adding index signature to creating the Any interfaces, like this:

{% if definition.additionalProperties %}
    /**
     * {{ definition.description | trim }} with additional properties
     */
    export interface Any{{ definition.name | caseUcfirst }}{{ definition.name | getGenerics(spec, true) | raw }} extends {{ definition.name | caseUcfirst }}{{ definition.name | getGenerics(spec, false) | raw }} {
        [key: string]: {% if definition.additionalProperties.type %}{{ definition.additionalProperties | getType(spec) }}{% else %}any{% endif %};
    }

{% endif %}

then we can add = Any{{ definition.name | caseUcfirst }}{{ definition.name | getGenerics(spec, true) | raw }} to all the required methods.

We also need to take care of cases where the entire interface is just using the index signature like:

/**
 * Preferences
 */
type Preferences = {
  [key: string]: any;
};

then we don't want the any decalaration.

@loks0n can you confirm the naming for this? currently we just have index signature for Document and Preferences so the change will only be applied for AnyDocument

[ChiragAgg5k]

Opening this for contributions. Feel free to check out the repo and raise a pr! https://github.com/appwrite/sdk-generator/