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

fix: format create DB response #12

Merged
merged 3 commits into from
Jan 23, 2024
Merged

Conversation

kevinmitch14
Copy link
Contributor

Currently, the return value of turso.databases.create() includes PascalCase keys, which doesn't match the types for the method.

This is the current response when creating a new DB.

{
    DbId: <database-id>,
    Hostname: <hostname>,
    IssuedCertCount: 0,
    IssuedCertLimit: 0,
    Name: <name>
} 

This is what TS thinks is returned.
Screenshot 2024-01-22 at 16 19 29

The changes I made will only convert keys to snakeCase, but the types are still incorrect. Eg. version is not returned but TS thinks it is.

More than happy to make further changes, as this is just a quick patch really.

@notrab
Copy link
Member

notrab commented Jan 22, 2024

Good find @kevinmitch14! Thanks so much for the PR.

We want to make the API return snakeCase in the future, so this temp formatter helps advocate for that field use in the client. We'll probably add the snakeCase value to current API responses soon and deprecate the old ones over time...

@kevinmitch14 kevinmitch14 marked this pull request as ready for review January 22, 2024 22:07
@notrab
Copy link
Member

notrab commented Jan 23, 2024

@kevinmitch14 I pushed a few changes to the branch that formats the create response.

I was initially going to use Partial<Database> but that required the use of hasOwnProperty to properly format the response. Since TS doesn't like hasOwnProperty in my experience, I decided to simplify things by creating another formatter. This is likely to be removed anyways once our API returns the updated response.

@notrab
Copy link
Member

notrab commented Jan 23, 2024

@kevinmitch14 if you're happy with the changes I'll merge this today :shipit:

src/database.ts Outdated Show resolved Hide resolved
@notrab notrab merged commit 97bd3eb into tursodatabase:main Jan 23, 2024
Copy link

🎉 This PR is included in version 1.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

2 participants