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

Add return type definitions to datasources #63

Merged
merged 1 commit into from
Dec 3, 2019

Conversation

mtgto
Copy link
Collaborator

@mtgto mtgto commented Dec 1, 2019

Define return types for DataSources.

I plan to add typedef for charts, queries and React components (Props and State).


abstract cancel();
// @todo Set return type as Promise<void> ?
abstract cancel(): void | Promise<void>;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BigQuery, MySQL and PostgreSQL uses Promise, but other not use.
I suggest to every datasource returns Promise<void>.


abstract connectionTest();
// @todo Define type of the result (boolean or void ?)
abstract connectionTest(): Promise<any>;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Datasources have different return value type.
(true or void)
I suggest to use Promise<void>.

return this._execute(query);
}

async cancel() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sqlite_interrupt seems to be synchronized.
TryGhost/node-sqlite3#518

}

async fetchTables() {
Copy link
Collaborator Author

@mtgto mtgto Dec 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: If we update @google-cloud/bigquery to latest, we can use its type definition for typescript.
https://github.com/googleapis/nodejs-bigquery

(But I don't have the environment for testing)

export type ConfigSchemaType = {
readonly name: string;
readonly label: string;
readonly type: string;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be expressed like:

readonly type: "radio" | "checkbox" | "password" | "string" | "number";

I prefer to use union type.

@hokaccha hokaccha merged commit fff4008 into bdash-app:master Dec 3, 2019
@hokaccha
Copy link
Member

hokaccha commented Dec 3, 2019

Great works, thanks!

@mtgto mtgto deleted the base_type branch January 18, 2020 12:25
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.

2 participants