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

Request for TypeScript support #29

Closed
c-kirkeby opened this issue Sep 14, 2018 · 29 comments
Closed

Request for TypeScript support #29

c-kirkeby opened this issue Sep 14, 2018 · 29 comments
Labels
good first issue Good for newcomers

Comments

@c-kirkeby
Copy link

This project looks fantastic and it would be great to get TypeScript support as well. TypeScript has quite a big community but with this being quite a new project there doesn't appear to be a community definition yet. I'd be happy to have a crack at helping with this, but I have not written an index.d.ts definition before. Microsoft have a guide over at https://www.typescriptlang.org/docs/handbook/declaration-files/by-example.html.

@radex
Copy link
Collaborator

radex commented Sep 14, 2018

TypeScript support would be cool

We don't use it at @Nozbe, but feel free to send a pull request with the definition. We'll be happy to accept it

@radex radex added the good first issue Good for newcomers label Sep 17, 2018
@kroltan
Copy link

kroltan commented Sep 17, 2018

Started working on it today, already typed most of the modules imported by index.js.
https://github.com/kroltan/WatermelonDB/tree/typescript

Some stuff doesn't even really compile yet, I was just transliterating the Flow to TS. I'll have to find a replacement for Class<T>

@radex
Copy link
Collaborator

radex commented Sep 18, 2018

Good stuff @kroltan! Thanks for working on this.

@kroltan
Copy link

kroltan commented Sep 25, 2018

Status update: Most of it has been typed, including all of the decorators (it was tricky to express that with strong types in TS, but thanks to the Reactiflux community a good soluton was found).

Only thing left now are the modules in src/utils.

Now that it's usable, I'll begin integrating WatermelonDB on our current project to validate if the typings are correct in practice.

Besides that, it is still missing a code style cleanup pass, since my IDE did not recognize this project's style for whatever reason, so it's a bit of a mishmash of quotings and semicolons.

@radex
Copy link
Collaborator

radex commented Sep 25, 2018

Only thing left now are the modules in src/utils.

Why do you need this? This would only be necessary if you want to import Watermelon utils for your own use, and I don't think we talk in the documentation about that (do we?)

@kroltan
Copy link

kroltan commented Sep 25, 2018

I don't! I was just plowing through every single module and typing them. Then scratch that part :)

@venkatd
Copy link

venkatd commented Oct 1, 2018

@kroltan I think the best approach here would be to only add typescript definitions to things accessed by the public api. That way if internals change, it won't be necessary to rework the public api definitions.

The main win here is someone who is using typescript is gets great code completion.

Thanks for the work here!

@radex
Copy link
Collaborator

radex commented Oct 2, 2018

That way if internals change, it won't be necessary to rework the public api definitions.

That makes sense. Internals change frequently, and most of that is invisible to normal use of Watermelon.

@Aidurber
Copy link
Contributor

I'm happy to update the definitions for the DatabaseProvider and withDatabase HoC when the initial TS definitions and my PR land, unless @kroltan wants to pick them up. It's literally just a database:Database prop.

@kroltan
Copy link

kroltan commented Oct 11, 2018

Heyo! Sorry, have been rather busy with life stuff lately, couldn't pick this up. Things have cleared up though.
I'm hoping for some time for it on Sunday, the only thing left to do is to delete the internals typings, and double-check the packaging.

@radex
Copy link
Collaborator

radex commented Nov 9, 2018

Hey! Did anyone push this effort forward? We can adopt TypeScript in chunks, we don't have to do it all at once, so I encourage you all to post pull requests even if they only add partial support.

@kroltan @Aidurber @venkatd @bsr203

@bsr203
Copy link
Contributor

bsr203 commented Nov 9, 2018

I used @kroltan s typing, but had issue adapting as it had the internals. I too looking forward to the cleaned up version as it does a great job covering the API.

@radubatori
Copy link

I gave WatermelonDB a try for a couple of days, but in the end I went with PouchDB + MobX. However, I did write my own declarations in the process. They are not complete and can be improved quite a bit, but they may help if anybody is willing to extend them.

declare enum ColumnType {
  'string' = 'string',
  'number' = 'number',
  'boolean' = 'bool'
}

declare enum AssociationType {
  'belongs_to' = 'belongs_to',
  'has_many' = 'has_many'
}

declare module '@nozbe/watermelondb' {
  import SQLiteAdapter from '@nozbe/watermelondb/adapters/sqlite';
  import { Observable } from 'rxjs';

  class Database {
    constructor(params: { adapter: SQLiteAdapter; modelClasses: Function[] });
    readonly collections: CollectionMap;
    readonly adapter: SQLiteAdapter;
    readonly schema: AppSchema;
  }

  interface CollectionMap {
    get<T extends Model>(tableName: string): Collection<T>;
  }

  interface Collection<T extends Model> {
    database: Database;
    table: string;
    schema: TableSchema;

    find(id: string): Promise<T>;
    findAndObserve(id: string): Observable<T>;
    query(...conditions: Condition[]): Query<T>;
    create(recordBuilder: (record: T) => void): Promise<T>;
  }

  // ------ QueryDescription ------
  type NonNullValue = number | string | boolean;
  type NonNullValues = number[] | string[] | boolean[];
  type Value = NonNullValue | null;
  type Operator = 'eq' | 'notEq' | 'gt' | 'gte' | 'weakGt' | 'lt' | 'lte' | 'oneOf' | 'notIn' | 'between';
  type ComparisonRight = { value: Value } | { values: NonNullValues } | ColumnDescription;
  interface Comparison {
    operator: Operator;
    right: ComparisonRight;
  }
  interface ColumnDescription {
    column: string;
  }
  interface WhereDescription {
    type: 'where';
    left: string;
    comparison: Comparison;
  }

  type Where = WhereDescription | And | Or;
  type And = { type: 'and'; conditions: Where[] };
  type Or = { type: 'or'; conditions: Where[] };
  type On = {
    type: 'on';
    table: string;
    left: string;
    comparison: Comparison;
  };
  type Condition = Where | On;
  type QueryDescription = { where: Where[]; join: On[] };

  class Q {
    static eq(valueOrColumn: Value | ColumnDescription): Comparison;
    static notEq(valueOrColumn: Value | ColumnDescription): Comparison;
    static gt(valueOrColumn: NonNullValue | ColumnDescription): Comparison;
    static gte(valueOrColumn: NonNullValue | ColumnDescription): Comparison;
    static weakGt(valueOrColumn: NonNullValue | ColumnDescription): Comparison;
    static lt(valueOrColumn: NonNullValue | ColumnDescription): Comparison;
    static lte(valueOrColumn: NonNullValue | ColumnDescription): Comparison;
    static oneOf(values: NonNullValues): Comparison;
    static notIn(values: NonNullValues): Comparison;
    static between(left: number, right: number): Comparison;
    static where(left: string, valueOrComparison: Value | Comparison): WhereDescription;
    static and(...conditions: Where[]): And;
    static or(...conditions: Where[]): Or;
    static on(table: string, column: string, value: Value): On;
    static on(table: string, column: string, comparison: Comparison): On;
    static on(table: string, where: WhereDescription): On;
  }
  // ------ end QueryDescription ------

  interface Query<T extends Model> {
    collection: Collection<T>;
    description: QueryDescription;

    extend(...conditions: Condition[]): Query<T>;
    fetch(): Promise<T[]>;
    fetchCount(): Promise<number>;
    observe(): Observable<T[]>;
    observeWithColumns(rawFields: string[]): Observable<T[]>;
    observeCount(isThrottled: boolean): Observable<number>;
    markAllAsDeleted(): Promise<void>;
    destroyAllPermanently(): Promise<void>;
  }

  type BelongsToAssociation = { type: 'belongs_to'; key: string };
  type HasManyAssociation = { type: 'has_many'; foreignKey: string };
  type AssociationInfo = BelongsToAssociation | HasManyAssociation;
  type SyncStatus = 'synced' | 'created' | 'updated' | 'deleted';

  abstract class Model {
    static table: string;
    static associations: Record<string, AssociationInfo>;

    readonly id: string;
    readonly syncStatus: SyncStatus;
    readonly collection: Collection<this>;

    update(recordUpdater: (object: this) => void): Promise<void>;
    prepareUpdate(recordUpdater: (object: this) => void): this;
    markAsDeleted(): Promise<void>;
    destroyPermanently(): Promise<void>;
    observe(): Observable<this>;
  }

  interface Relation<T extends Model> {
    id: string;

    fetch(): Promise<T>;
    set(record: T): void;
    observe(): Observable<T>;
  }

  // ---------- Schema -----------
  interface AppSchema {
    version: number;
    tables: Record<string, TableSchema>[];
  }

  interface TableSchema {}

  interface ColumnSchema {
    name: string;
    type: ColumnType;
    isOptional?: boolean;
    isIndexed?: boolean;
  }

  interface SchemaMigrations {}

  export function appSchema(schema: { version: number; tables: TableSchema[] }): AppSchema;

  export function tableSchema(schema: { name: string; columns: ColumnSchema[] }): TableSchema;
  // ---------- end Schema -----------
}

declare module '@nozbe/watermelondb/adapters/sqlite' {
  import { AppSchema, SchemaMigrations } from '@nozbe/watermelondb';

  export default class SQLiteAdapter {
    constructor(params: { dbName: string; schema: AppSchema; migrationsExperimental?: SchemaMigrations });
  }
}

declare module '@nozbe/watermelondb/decorators' {
  export function field(columnName: string): any;
  export function date(columnName: string): any;
  export function relation(columnName: string, foreignKey: string): any;
  export function children(columnName: string): any;
}

declare module '@nozbe/with-observables';

@radex
Copy link
Collaborator

radex commented Nov 13, 2018

Nice work @radubatori ! Thank you!

Does anybody want to clean it up a bit and send a pull request to add this to 🍉?

@radubatori PS. If you can spare a couple of minutes, send me an email — I'd be curious to understand your reasons for going with PouchDB+MobX over Watermelon :)

@jakebloom
Copy link
Contributor

Would love to see this merged at some point - @kroltan if you're too busy, can you email me with what you have so far and I can take a look?

@radex
Copy link
Collaborator

radex commented Jan 11, 2019

@jakebloom I believe it's in this repo:
https://github.com/kroltan/WatermelonDB/tree/typescript — but it's possible @kroltan has some unpushed changes.

Would greatly appreciate PRs!

@radex
Copy link
Collaborator

radex commented Feb 26, 2019

@jakebloom @kroltan Hey, any update on this? I know a lot of people would really like 🍉 to have TypeScript types. It doesn't have to be perfect! Some types are better than none! (Since other people will be more willing to contribute if some work is already done)

@jakebloom
Copy link
Contributor

@radex I really want to put some time into this but unfortunately I'm just too busy at the moment. When things quiet down for me I'll certainly have a look

@kilbot
Copy link
Contributor

kilbot commented Mar 1, 2019

I merged @kroltan's typings into #269 and have been using them in my project for the last couple of days, they're almost complete (thanks for all your work @kroltan!) ... it's so good to have code completion for WatermelonDB.

usedatabase-ts

There are a couple of issues that need to be fixed before it can be released, but my understanding of TypeScript is basic so I could use any advice/assistance.

  • the modelClasses on new Database({ adapter, modelClasses }) is complaining because of subclassing
  • the decorators are not completely typed

@kilbot
Copy link
Contributor

kilbot commented Mar 2, 2019

If anyone would like to help test the typings before release, please download this file to your project root directory. Your linter should pick it up automatically, but you may have to adjust your tsconfig.

Problem areas are:

  • L125 - how to allow SubClass extends Model?
  • L819 - I'm not sure where @kroltan was going with this 😂

@bsr203
Copy link
Contributor

bsr203 commented Mar 29, 2019

@kilbot thanks for your effort on this. Everything works for me except errors like

error TS2345: Argument of type '(m: c) => void' is not assignable to parameter of type '(record: Model) => void'.
  Types of parameters 'm' and 'record' are incompatible.
    Type 'Model' is missing the following properties from type 'Message': text, status, and 2 more.

54   return db.collections.get(TABLE).create((m: Message) => {
                                             ~~~~~~~~~~~~~~~~~~~~~~

I guess this is what you refer as how to allow SubClass extends Model?. Is it possible to make it work with typed methods like create. Just thinking out loud.

thanks again.

@kilbot
Copy link
Contributor

kilbot commented Mar 30, 2019

Hi @bsr203, yes there is something fundamentally wrong with the Model definition or the way that 'Subclass extends Model' is referenced. I have read through definition files from other libraries and tried various fixes, but ultimately I'm just guessing at a solution ... my knowledge of typescript is too limited at the moment.

I would love @kroltan to rejoin the conversation if possible. It would be helpful to know if the typescript definitions were simply ported from the existing flow types - in which case there may be some underlying difference between the two systems which needs to be fixed.

@kroltan
Copy link

kroltan commented Mar 30, 2019

Hello! Sorry for abandoning this issue, life happens. Now I've got time to follow the discussion, if not to contribute myself.

@kilbot yes, I basically rearranged the flow typings into TS, but tried to port over anything that errored out manually with lots of googling. Record extends Model being one of them.

As for L819, the idea is that a decorator is like a raw decorator factory, but which might return either a raw decorator or a descriptor. I do not recall why I did it that way :/

Also. for the CollectionMap#get to work properly, I believe it has to either be invoked with a type parameter, or have your collection name be a constant typed as a TableName instead of just a plain string. That is, either:

const collection = db.collections.get<Table>("table");

or

const TABLE: TableName<Table> = "table";
const collection = db.collections.get(TABLE);

Advantage of second form is reduced typing (as in button-pressing, not typescript 😄) on the call site.

I have not tested either however, since I don't have a Node development environment anymore.

@bsr203
Copy link
Contributor

bsr203 commented Mar 30, 2019

thanks @kilbot I too not an expert, but what I was referring typed methods is (generic / parametric)

export interface StoreMessage extends Model {
  id: string;
  ...
}

export default class Message<StoreMessage> {
..
}

so the model class and methods are parametrized with T supplied by user. I am not sure about it either :-)

@kilbot
Copy link
Contributor

kilbot commented Mar 30, 2019

Hi @kroltan! Thanks for the reply 😄

I've created a sandbox where we can isolate and work on some of these issues: https://codesandbox.io/s/0m028yz8zn

The SubClass extends Model problem I am seeing is shown below, I believe it's the same underlying problem which is being seen by @bsr203.

Screen Shot 2019-03-30 at 11 55 47 pm

@bsr203, if I understand correctly, you are suggesting we export a Model interface which can be extended? Or perhaps we could make the Model class itself a generic?

@bsr203
Copy link
Contributor

bsr203 commented Mar 30, 2019

@kilbot ya, model class itself generic, where the type is specified by the user.

An example is how apollo specify expected query data during query declaration
https://www.apollographql.com/docs/react/recipes/static-typing#typing-components
https://github.com/apollographql/react-apollo/blob/master/src/Query.tsx#L85

@kroltan
Copy link

kroltan commented Mar 30, 2019

I don't think we can use a generic Model here, because there are situations where we must refer to the type of "some model", not a specific model. So we would end up with the same situation as currently, but with more angle brackets.

@stale
Copy link

stale bot commented Jun 28, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Jun 28, 2019
@jakebloom
Copy link
Contributor

@c-kirkeby @radex we can close this without the wontfix label, there are issues with the typescript definitions as they stand, but a "request for support" has largely been fulfilled

@stale stale bot removed the wontfix This will not be worked on label Jun 28, 2019
@radex radex closed this as completed Jul 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

9 participants