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

web - add index type to Models that have additionalProperties set to … #457

Merged
merged 3 commits into from
May 31, 2022

Conversation

nartc
Copy link
Contributor

@nartc nartc commented May 27, 2022

…true

Current behavior

In the web SDK typings, Preferences is a type Preferences = {} which makes it tricky to work with in projects with TypeScript strict mode on.

Expected behavior

type Preferences should be an indexable type or a Record.

type Preferences = Record<string, any>;
// or
type Preferences = {
   [key: string]?: any;
}

This PR adds an indexable record to a model under namespace Models if its definition has additionalProperties: true

Additional information

I'm not sure how to verify the change with the current tests setup of this repo. Any suggestions would be much appreciated

@brandonroberts
Copy link

@TorstenDittmann @christyjacob4 we had a conversation about this PR already to allow more flexibility with the types. It will help Angular users better extend the base types.

@nartc
Copy link
Contributor Author

nartc commented May 27, 2022

@brandonroberts To clarify, in order to extend the type, the type itself has to be an interface because Declaration Merging only works with Interfaces. The SDK currently uses Type so to stay with that, I just make Preferences an indexable type so that the consumers can access any property they need.

Today, we can do the following:

Models.User<{bio: string, avatar: string}>;

In case where we want to quickly access a property without having to specify a custom Preferences type, this PR allows that

@TorstenDittmann
Copy link
Contributor

@TorstenDittmann @christyjacob4 we had a conversation about this PR already to allow more flexibility with the types. It will help Angular users better extend the base types.

I don't quite remember how we handle types in the preferences, it's possible that the values only allow strings.

templates/web/src/sdk.ts.twig Outdated Show resolved Hide resolved
@nartc nartc requested a review from TorstenDittmann May 30, 2022 17:23
@TorstenDittmann TorstenDittmann merged commit 1cdfedb into appwrite:master May 31, 2022
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

Successfully merging this pull request may close these issues.

3 participants