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: releaseConnection types and promise #2053

Merged
merged 6 commits into from
Jun 11, 2023

Conversation

wellwelwel
Copy link
Sponsor 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
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.

1 participant