fix: update pool cluster of to use pool namespace

[JacobBanghart]

pool_cluster.js was out of sync with the typing and returns a pool namespace rather than a self-reference pool cluster

fixes: #1991
This should fix one of the issues. There are some additional fixes that can be made such as having more dedicated typing for the union types.

[wellwelwel]

@JacobBanghart, in JS everything looks fine, but when I tried this changes in TS, it works in mysql2 import, but not in mysql2/promise.

Just a basic test for each import way:

---

• mysql2 ✅
  • JS
  • TS

import mysql from 'mysql2';

const poolCluster = mysql.createPoolCluster();

poolCluster.add('MY_CLUSTER', {
   host: '',
   user: '',
   password: '',
   database: '',
});

// from: getConnection
{
   poolCluster.getConnection('MY_CLUSTER', (err, conn) => {
      conn.query('SELECT * FROM `table`;', (err, rows) => {
         console.log(rows);
      });
   });
}

// from: of
{
   const MY_CLUSTER = poolCluster.of('MY_CLUSTER');

   MY_CLUSTER.query('SELECT * FROM `table`;', (err, rows) => {
      console.log(rows);
   });
}

---

• mysql2/promise ❌
  • JS
  • TS

import mysql from 'mysql2/promise';

(async () => {
   const poolCluster = mysql.createPoolCluster();

   poolCluster.add('MY_CLUSTER', {
      host: '',
      user: '',
      password: '',
      database: '',
   });

   // from: getConnection
   {
      const MY_CLUSTER = await poolCluster.getConnection('MY_CLUSTER');
      const [rows] = await MY_CLUSTER.query('SELECT * FROM `table`;');
      console.log(rows);
   }

   // from: of
   {
      const MY_CLUSTER = poolCluster.of('MY_CLUSTER');
      const [rows] = await MY_CLUSTER.query('SELECT * FROM `table`;');
      console.log(rows);
   }
})();

• The errors occur because in both import methods the typings are called from callback (index.d.ts).

---

If these changes are compatible with both import methods, I think basic tests could be created to ensure that every new change respects the basic usage of the createPoolCluster.

Currently, this tests covers the most common cases of query and execute in createConnection and createPool.

You can see a practical example here: test:tsc-build

[sidorares]

@wellwelwel would you like to join as a co-maintainer? Happy to give you write permissions

[JacobBanghart]

So I had issues getting your second example to work on the latest version. It is having other issues unrelated to my change. But thanks for pointing out the relation to the index.d.ts. It seems more practical to place this as a part of the namespace definition. Then I do not modify anything but raw types and use an interface instead. The biggest disadvantage that I see to this is that you are not directly exporting the PoolNamespace for usage, but I can not think of a given use case for exporting it.

[wellwelwel]

would you like to join as a co-maintainer? Happy to give you write permissions

@sidorares, It would be a pleasure. If you prefer, I can reach out to you via email or a most convenient way for you.

• I sent you an email 🙋🏻‍♂️

[wellwelwel]

@JacobBanghart, when you do it in PoolCluster.d.ts:

export interface PoolNamespace {
  getConnection(callback: (err: NodeJS.ErrnoException | null, connection: PoolConnection) => any): void;
  // ...
}
// ...
declare class PoolCluster extends EventEmitter {
  of(pattern: string, selector?: string): PoolCluster.PoolNamespace;
  // ...
}

Naturally this will affect both mysql2 and mysql2/promise and by this way, the typings in PoolNamespace are appropriate for mysql2 (because it expects for callbacks), but not for mysql2/promise.

---

You can compare these "import" differences, by example, looking at how it works for mysql2 in:

node-mysql2/index.d.ts

Line 89 in db91252

export interface Pool extends mysql.Connection {

And how it changes for mysql2/promise in:

node-mysql2/promise.d.ts

Line 91 in db91252

export interface Pool extends EventEmitter {

Then, adapting how it would be for the createPoolCluster.

---

I understand that you didn't modify the createPoolCluster itself, but the of option within it.

The errors in my mysql2/promise example after the const MY_CLUSTER = poolCluster.of('MY_CLUSTER') are simple tests to ensure that the typings are compatible for both use cases.

---

If you don't mind, I can help you with this 🙋🏻‍♂️

[sidorares]

@wellwelwel small suggestion - when you link a code line, please link to a commit, not to a branch head ( press y to update url to include commit. ), otherwise change to a line above can make you link to incorrect line

[wellwelwel]

Closing in favor of #2084.