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

Inconsistent camelCase use for datetime in schema #3944

Closed
rijkvanzanten opened this issue Jul 23, 2020 · 3 comments · Fixed by #4589
Closed

Inconsistent camelCase use for datetime in schema #3944

rijkvanzanten opened this issue Jul 23, 2020 · 3 comments · Fixed by #4589

Comments

@rijkvanzanten
Copy link
Collaborator

Environment

Knex version: 0.21.2

Bug

  1. Explain what kind of behaviour you are getting and how you think it should do

The docs describe creating a datetime column by using table.datetime(): http://knexjs.org/#Schema-datetime

However, the TypeScript definition and tests seem to indicate it should be table.dateTime():

dateTime(columnName: string, options?: Readonly<{useTz?: boolean, precision?: number}>): ColumnBuilder;

table.dateTime('dateTimeCol');

as a matter of fact, I was only able to find a single reference that seems to use datetime in schema building:

this.datetime('foo', 6);

Are the docs wrong, or is there something bigger here I'm missing? (Also, how does that last test pass when using datetime() instead of dateTime()? Are both valid? @lorefnon

@lorefnon
Copy link
Collaborator

The same method is aliased as datetime and dateTime.

The TS defs should mention both - but currently doesn't.

@rijkvanzanten
Copy link
Collaborator Author

Ahh I see. Thanks for the heads up @lorefnon 👍

Shall I open a PR to extend the index.d.ts file?
Do we want to update the docs to mention these aliases somewhere consistent?

@lorefnon
Copy link
Collaborator

@rijkvanzanten Sure. That would be cool. The knex docs are maintained in this repository, and can be updated there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants