Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: releaseConnection types and promise #2053

Merged
merged 6 commits into from
Jun 11, 2023

Conversation

wellwelwel
Copy link
Collaborator

@wellwelwel wellwelwel commented Jun 11, 2023

This fixes the #1761 and #1563.


JS

The promise.js didn't have the method releaseConnection.

Instead of creating from scratch, I just "extended" it from the release method:

releaseConnection(connection) {
  if (connection instanceof PromisePoolConnection) connection.release();
}

Then, I added the test to test/integration/promise-wrappers/test-promise-wrappers.js.


TS

I had to fix a few wrong types before to fix the releaseConnection.

Before this PR, according by typings, this should be possible:

await (await (await pool.getConnection()).getConnection()).getConnection();
// To infinity and beyond... 😵‍💫

So, I fixed it to only Pool have the method getConnection.

  • At this time, I added "negative" tests in tsc-build to ensure that the above code doesn't occur again.

Lastly, I added some tests for each method that I changed something in types:

  • releaseConnection
  • release
  • getConnection
  • connection

@wellwelwel wellwelwel merged commit 855c43c into sidorares:master Jun 11, 2023
@wellwelwel wellwelwel deleted the release-connection branch June 11, 2023 08:59
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant