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.

Sign up for GitHub

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

Jump to bottom

Make TS stubs modern #3080

Merged
merged 1 commit into from Mar 5, 2019
Merged

Make TS stubs modern #3080

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/migrate/stub/ts.stub
View file
Open in desktop
Expand Up @@ -2,20 +2,20 @@ import * as Knex from "knex";

<% if (d.tableName) { %>
export async function up(knex: Knex): Promise<Knex.SchemaBuilder> {
return knex.schema.createTable("<%= d.tableName %>", function (t: Knex.AlterTableBuilder) {
return knex.schema.createTable("<%= d.tableName %>", (t: Knex.AlterTableBuilder) => {
t.increments();
t.timestamps();
});
};
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why remove semicolons, though?

Copy link
Member

Choose a reason for hiding this comment

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

Normal function declaration shouldn't have semicolon there. With arrow function, it would be obligatory though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@felixmosh So if I understand correctly, semicolons should be restored, since it's an arrow function?..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kibertoad , no, up is not an arrow function :]
The function that we pass to createTable is an arrow func, and there are semi-colons there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fair enough.

<% } else { %>
export async function up(knex: Knex): Promise<any> {
};
}
<% } %>
<% if (d.tableName) { %>
export async function down(knex: Knex): Promise<Knex.SchemaBuilder> {
return knex.schema.dropTable("<%= d.tableName %>");
};
}
<% } else { %>
export async function down(knex: Knex): Promise<any> {
};
}
<% } %>
4 changes: 2 additions & 2 deletions src/seed/stub/ts.stub
View file
Open in desktop
@@ -1,9 +1,9 @@
import * as Knex from "knex";

exports.seed = function (knex: Knex): Promise<any> {
export async function seed(knex: Knex): Promise<any> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if it returns promise anyway, why make it async?

Copy link
Member

Choose a reason for hiding this comment

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

To allow using await inside the function.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Await inside the function is kinda broken, though, not sure we want to encourage it: #2581

Copy link
Member

Choose a reason for hiding this comment

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

Yes we do :) That is really strange though. It doesn't make sense at all that it works when calling then(), but not with await... Maybe we have missed returning a promise from .then somewhere 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll try writing a failing test for that case.

// Deletes ALL existing entries
return knex("table_name").del()
.then(function () {
.then(() => {
// Inserts seed entries
return knex("table_name").insert([
{ id: 1, colName: "rowValue1" },
Expand Down