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/query signature #60

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

angelsvirkov
Copy link

@angelsvirkov angelsvirkov commented Oct 22, 2021

Applying a fix for #59 . In other words, this pull request enables the usage of connection.query and similar methods with all possible mysql function signatures. This makes it possible to pass an options object to connection.query. Previously this was not possible because the function signature was fixed to only one case.

Implementation details:

  • Making use of the mysql util function createQuery. This function takes all possible signatures and produces the corresponding values by handling all cases as can be seen here. Moreover, the function already returns the input if it is already an instance of Query (already executed createQuery() )
  • Because of this instanceof Query check, I had to turn the mysql dependency to a dev dependency and to a peer dependency. In this way we make sure that the instance of the Query is the same instance that the consumers of this package are going to use. Especially important whenever the mysql package is occurring multiple times in the dependency tree of a consumer package.
  • I had to apply /* eslint-disable no-use-before-define */ because the new implementation explicitly makes use of the variable hoisting to make the callback wrappers work.
  • I have manually tested this new implementation in my own private repo which has multiple dependencies of mysql and node-mysql-utilities.

Checks:

  • tests and linter show no problems (npm t)
  • tests are added/updated for bug fixes and new features
  • code is properly formatted (npm run fmt)

- Issue: Calling `connection.query` with an options object fails after a connection has been upgraded tshemsedinov#59
- Calling `connection.query` with an options object fails after a connection has been upgraded tshemsedinov#59
- Issue: Calling `connection.query` with an options object fails after a connection has been upgraded tshemsedinov#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