-
Notifications
You must be signed in to change notification settings - Fork 155
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
feat: typescript definitions for mysql2 lib #676
base: master
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #676 +/- ##
=======================================
Coverage 83.56% 83.56%
=======================================
Files 36 36
Lines 1813 1813
=======================================
Hits 1515 1515
Misses 298 298 ☔ View full report in Codecov by Sentry. |
packages/mysql/lib/mysql_p.d.ts
Outdated
}; | ||
|
||
interface PatchedMySQL2CreateConnectionFunction { | ||
(config: MySQL2.ConnectionOptions | string): Promise<PatchedConnection>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default, we should also support mysql2.createConnection
that returns a Connection
. Our instrumentation does support a return value of a Promise<Connection>
, but that requires supporting TS definitions of patched mysql2/promise
rather than mysql2
, which only the latter is supported by this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've started to de-scope this PR to support return of Connection
only, rather than extending it to support Promise<Connection>
: 8d4597f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jj22ee for calling out the distinction between mysql2
and mysql2/promise
, which I didn't quite appreciate on the outset.
I'd be happy to explore an extension of X-Ray's type definitions for the latter in future, but I'll keep this PR specific to mysql2
for the time being.
c4851ea
to
325bde5
Compare
Closes #387.
Extending the AWS X-Ray MySQL library to support patching of MySQL2 connections, pools, and pool clusters.
Type definitions are derived for the most part from index.d.ts with thanks to @jj22ee for their valuable feedback on this PR.
There's scope to extend this library further to support mysql2/promise, allowing for usage of promises rather than the callbacks that this PR focuses on.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.