Skip to content

Commit

Permalink
fix: circular dependencies (#3081)
Browse files Browse the repository at this point in the history
* fix(lib): circular dependency index.js <-> lib

pool.js and pool_connection.js required index.js, and
index.js required pool.js and pool_connection.js.

Circular dependency can cause all sorts of problems.
This fix aims to provide a step towards fixing a problem with
@opentelemetry/instrumentation-mysql2, which looses several
exports when using its patched require.

For instance, `format` is lost and can cause an instrumented
application to crash.

* fix: circular dependency index.js <-> promise.js

index.js and promise.js require each other.

Circular dependency can cause all sorts of problems.
This commit aims to fix a problem with
@opentelemetry/instrumentation-mysql2, which looses several
exports when using its patched require.

For instance, `format` is lost and can cause an instrumented
application to crash.

* refactor: split common.js into lib modules

The proposal to put common.js into lib was good, but it felt
weird to arbitrarily stuff just some exported functions in
lib/common.js. This made sense when common.js was meant
to provide commons for index.js and promise.js. But
in the lib, this felt like a weird scope.

I therefore split common.js into

- lib/create_connection.js
- lib/create_pool.js
- lib/create_pool_cluster.js

Also made `require` more consistent in all affected files: all
`require` files now have a js suffix when they refer to a
single local file.

* fix: circular dependency to promise.js

promise.js was required by lib sources, and promise.js required
the same lib sources eventually. Extracted the respective
classes to lib. Also extracted functions that are
shared between several of the new files.

Decided to put each exported class / function into its own file.
This may bloat the lib folder a bit, but it provides clarity
(where to find what).

* fix: missing patched functions

The extraction of classes was performed without extracting
the patch of methods like `escape`, etc.
The patching is now performed in the files that define the
respective classes, guaranteeing patched versions when
these files are required.

* chore: remove unused require

* style: add missing semicolons and file name suffixes for require

Co-authored-by: mknj <[email protected]>

* chore: resolve all remaining circular dependencies

* chore: better organize new files

---------

Co-authored-by: mknj <[email protected]>
Co-authored-by: wellwelwel <[email protected]>
  • Loading branch information
3 people authored Nov 13, 2024
1 parent 5615273 commit d5a76e6
Show file tree
Hide file tree
Showing 17 changed files with 1,782 additions and 1,689 deletions.
38 changes: 16 additions & 22 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,32 @@

const SqlString = require('sqlstring');

const Connection = require('./lib/connection.js');
const ConnectionConfig = require('./lib/connection_config.js');
const parserCache = require('./lib/parsers/parser_cache');
const parserCache = require('./lib/parsers/parser_cache.js');

exports.createConnection = function(opts) {
return new Connection({ config: new ConnectionConfig(opts) });
};
const Connection = require('./lib/connection.js');

exports.createConnection = require('./lib/create_connection.js');
exports.connect = exports.createConnection;
exports.Connection = Connection;
exports.ConnectionConfig = ConnectionConfig;

const Pool = require('./lib/pool.js');
const PoolCluster = require('./lib/pool_cluster.js');
const createPool = require('./lib/create_pool.js');
const createPoolCluster = require('./lib/create_pool_cluster.js');

exports.createPool = function(config) {
const PoolConfig = require('./lib/pool_config.js');
return new Pool({ config: new PoolConfig(config) });
};
exports.createPool = createPool;

exports.createPoolCluster = function(config) {
const PoolCluster = require('./lib/pool_cluster.js');
return new PoolCluster(config);
};
exports.createPoolCluster = createPoolCluster;

exports.createQuery = Connection.createQuery;

exports.Pool = Pool;

exports.PoolCluster = PoolCluster;

exports.createServer = function(handler) {
exports.createServer = function (handler) {
const Server = require('./lib/server.js');
const s = new Server();
if (handler) {
Expand All @@ -42,7 +36,7 @@ exports.createServer = function(handler) {
return s;
};

exports.PoolConnection = require('./lib/pool_connection');
exports.PoolConnection = require('./lib/pool_connection.js');
exports.authPlugins = require('./lib/auth_plugins');
exports.escape = SqlString.escape;
exports.escapeId = SqlString.escapeId;
Expand All @@ -51,33 +45,33 @@ exports.raw = SqlString.raw;

exports.__defineGetter__(
'createConnectionPromise',
() => require('./promise.js').createConnection
() => require('./promise.js').createConnection,
);

exports.__defineGetter__(
'createPoolPromise',
() => require('./promise.js').createPool
() => require('./promise.js').createPool,
);

exports.__defineGetter__(
'createPoolClusterPromise',
() => require('./promise.js').createPoolCluster
() => require('./promise.js').createPoolCluster,
);

exports.__defineGetter__('Types', () => require('./lib/constants/types.js'));

exports.__defineGetter__('Charsets', () =>
require('./lib/constants/charsets.js')
require('./lib/constants/charsets.js'),
);

exports.__defineGetter__('CharsetToEncoding', () =>
require('./lib/constants/charset_encodings.js')
require('./lib/constants/charset_encodings.js'),
);

exports.setMaxParserCache = function(max) {
exports.setMaxParserCache = function (max) {
parserCache.setMaxCache(max);
};

exports.clearParserCache = function() {
exports.clearParserCache = function () {
parserCache.clearCache();
};
Loading

0 comments on commit d5a76e6

Please sign in to comment.