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

Promise Pool events inherit original pool connection class, not promise pool #3238

Open
ashleybartlett opened this issue Nov 27, 2024 · 0 comments · May be fixed by #3321
Open

Promise Pool events inherit original pool connection class, not promise pool #3238

ashleybartlett opened this issue Nov 27, 2024 · 0 comments · May be fixed by #3321

Comments

@ashleybartlett
Copy link

When using a Promise Pool, and subscribing to events (e.g.pool.on('acquire', (connection) => {})) the connection object does not match the type.

In the types, the connection type is exposed as a Promise PoolConnection, where in reality there is no conversion from the original, and so it's actually a base PoolConnection type. Meaning if I want to use it, i have to call the Promise() helper, which isn't on the Promise PoolConnection type.

This is causing a bit of confusion on my end, and i'm not sure what the expected behaviour is? Should these be converted to PromisePoolConnections, or should the types be updated appropriately?

import { PoolConnection } from 'mysql2'
import mysql from 'mysql2/promise'

// this is a PromisePool
const pool = mysql.createPool(options);

// PoolConnection here is NOT a PoolConnection from the Promise lib
// but typescript thinks it is, and so is giving an error.

pool.on('acquire', async (connection: PoolConnection) => {
    await connection.promise().beginTransaction();
  });
ashleybartlett added a commit to ashleybartlett/node-mysql2 that referenced this issue Jan 13, 2025
This fixes a number of small type errors compared to the underlying code. This is likely a breaking change for some users as it would have required some working around.

Should fix sidorares#3238 and sidorares#2667
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants