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 typescript types #1028

Closed
wants to merge 1 commit into from
Closed

Conversation

mike-marcacci
Copy link
Contributor

  • Initial import of types from Felix Becker's repo
  • Update eslint to support typescript
  • Add stronger typing in places where any is used
  • Add missing exports

@FredericLatour
Copy link

@mike-marcacci Be aware that you should also include the files ( from this repository as well:

@mike-marcacci
Copy link
Contributor Author

mike-marcacci commented Oct 8, 2019

Yes, I actually began a rewrite of these types, based mostly on those.

@Sparticuz
Copy link

How's this going?

@nojvek
Copy link

nojvek commented Mar 5, 2020

This is sick. Just wondering what is blocking ? Is there anything I can do to help ?

@sidorares
Copy link
Owner

hey @mike-marcacci , thanks for starting this. Would you like somebody to pick up from where you stopped?

@Jolg42
Copy link

Jolg42 commented Apr 21, 2020

Hi!
I'm curious to know if this is still ongoing? 🙃

@sidorares
Copy link
Owner

@Jolg42 looks like not, any help is appreciated here

@FredericLatour
Copy link

FredericLatour commented Apr 21, 2020

Does mysql2 brings anything to mariadb-connector in terms of features or whatever?

I mean:

  • mysql has TS types but no promises.
  • mysql2 has promises but no TS types.
  • mariadb-connector has both.

Is there any objective reason one would select mysql or mysql2 over mariadb-connector.

PS: this is not intended to be provocative
PS2: sorry for the multiple posts. Something went wrong with Github.

@sidorares
Copy link
Owner

@FredericLatour not sure if mariadb connector supports prepared statements, looks like not

@mprast
Copy link

mprast commented Apr 29, 2020

@sidorares is there anything blocking this PR at the moment? yes, @mike-marcacci was going to refactor some stuff, but it looks like if you just land this as-is it gets you to parity with types/mysql2, right? (and then at least the types will be hosted on npm)

@mike-marcacci
Copy link
Contributor Author

Ah hi folks, so sorry for the silence: I did indeed get sidetracked from this as other priorities emerged. It's likely that I will need to pick this back up in a couple months (if nobody else does first), but can't in the immediate term.

@sidorares
Copy link
Owner

sidorares commented Apr 29, 2020

@mike-marcacci thanks for what you already contributed. Do you think we can merge what's there now and you continue when you have time with a separate PR?

@xfoxfu
Copy link

xfoxfu commented Jun 13, 2020

For those any in callback functions, such as listener: () => any, it may simple be replaced by unknown, like listener: () => unknown, or as TypeScript doc says, listener: () => void.

@sidorares sidorares marked this pull request as ready for review September 15, 2020 03:17
@Bidthedog
Copy link

I am desperate for this and would like to help. Is this the latest version of the TS changes, and can anyone confirm what is left to do?

@Sparticuz
Copy link

Agree, let's get this merged. It will be easier to fix any missing types after the fact as we run into them. There have been a few commits to https://github.com/types/mysql2 as well.

@Bidthedog
Copy link

Bidthedog commented Sep 17, 2020

I've pulled the remote repo locally and rebased / fixed merge conflicts. Want me to submit another PR as I don't have access to this source otherwise?

@Bidthedog
Copy link

Submitted fixed merge conflicts in #1204. You can close this PR.

Once you publish the next release let me know and I'll test the types and attempt to push some updates.

@sidorares sidorares closed this Sep 17, 2020
@sidorares
Copy link
Owner

tanks everyone. @Bidthedog this is in npm now as v2.2.0

@xfoxfu
Copy link

xfoxfu commented Sep 18, 2020

@sidorares I've upgraded the package, but found no .d.ts files in the package. By downloading https://registry.npmjs.org/mysql2/-/mysql2-2.2.0.tgz, I found the CHANGELOG.md is latest, but there is not a .d.ts file as in the repo.

@Bidthedog
Copy link

There are types in node_modules/@types/mysql, but the base .d.ts files aren't included. I'll investigate - I just resolved the conflicts, didn't put any time into figuring out how it all works :)

@sidorares
Copy link
Owner

oops, maybe we need to update files entry in package.json

@sidorares
Copy link
Owner

sidorares commented Sep 18, 2020

if you on it now @Bidthedog can you make a pr and I'll publish 2.2.1

actually nevermind, I'll do that now

@sidorares
Copy link
Owner

@xfoxfu I believe #1205 should fix it, will publish in few minutes

@sidorares
Copy link
Owner

@xfoxfu @Bidthedog published v2.2.1 which should include type files now

@Bidthedog
Copy link

Thanks, busy with "paid" work stuff atm, but will help out with future PRs once I get my head around the repo :)

@xfoxfu
Copy link

xfoxfu commented Sep 18, 2020

This fix speed is amazing! Thanks!

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.

9 participants