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

Add enableKeepAlive property to ConnectionOptions interface #2286

Closed
luisasalas opened this issue Nov 20, 2023 · 5 comments
Closed

Add enableKeepAlive property to ConnectionOptions interface #2286

luisasalas opened this issue Nov 20, 2023 · 5 comments

Comments

@luisasalas
Copy link

Noticed enableKeepAlive is missing on Connection.d.ts

@wellwelwel
Copy link
Collaborator

wellwelwel commented Nov 21, 2023

Comparing the ConnectionOptions and validOptions, both enableKeepAlive and keepAliveInitialDelay options are missing. Instead, they are in PoolOptions:

/**
* Enable keep-alive on the socket. (Default: true)
*/
enableKeepAlive?: boolean;
/**
* If keep-alive is enabled users can supply an initial delay. (Default: 0)
*/
keepAliveInitialDelay?: number;

@luisasalas
Copy link
Author

luisasalas commented Nov 21, 2023

@wellwelwel I assumed they should be since they are defined in ConnectionConfig?

this.enableKeepAlive = options.enableKeepAlive !== false;
this.keepAliveInitialDelay = options.keepAliveInitialDelay || 0;

@wellwelwel
Copy link
Collaborator

wellwelwel commented Nov 21, 2023

I assumed they should be since they are defined in ConnectionConfig?

@luisasalas, yes, considering the typings consistency with the source code, the ideal would be to move them from PoolOptions to ConnectionOptions.


Also:

enableKeepAlive: 1,

keepAliveInitialDelay: 1,

@wellwelwel
Copy link
Collaborator

Gotcha, I see it's default to true. However, when using this library in typescript, if we wanted to pass in false, would it be ignored since it's not defined in the typings?

It should work as usual ✅

When using TypeScript, it will compile the code to JavaScript on build, then the /lib/connection_config.js is what will be considered instead of the Connection.d.ts.

Also, the declaration types from MySQL2 are separated from source code (neither depends on the other to work individually). Then these issues are really important to keep the typings consistent with the source code 🧑🏻‍🔧

@wellwelwel
Copy link
Collaborator

Fixed in v3.6.4 🙋🏻‍♂️

Now it will work for both createConnection, createPool and createPoolCluster:

import mysql, { PoolOptions, ConnectionOptions } from 'mysql2';

const poolOptions: PoolOptions = {
  enableKeepAlive: true,
  keepAliveInitialDelay: 0,
}

const connectionOptions: ConnectionOptions = {
  enableKeepAlive: true,
  keepAliveInitialDelay: 0,
}

mysql.createConnection(connectionOptions);
mysql.createPool(poolOptions);
mysql.createPoolCluster().add(poolOptions);
  • The same for mysql2/promise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants