Feature Request: Lowercase dialect category inputs from user

[AstroGD]

Environment

Knex version: 0.15.2
Database + version: MySQL
OS: Linux Debian

Select applicable tempalate from below.
If issue is about oracledb support tag @ atiertant. For MSSql tag @ smorey2 .
Rest of dialects doesn't need tags.

Bug

If you try to install Knex on Linux Debian the following error occurres: Error: Cannot find module './dialects/MySQL/index.js'

This due to the fact, that paths are case-sensitive and the path must be /dialects/MySQL/index.js

I highly reccomend you to either change the path in code or rename the folder, as an installation of knex on debian is currently impossible without changing the folder manually.

[ricardograca]

I can install knex just fine on Ubuntu 16.04: npm install knex.

Can you share what steps you took to install knex that produce the error you mention?

[AstroGD]

I typed npm install knex and then run my program. My program then crashed with an unhandled promise rejection because the file was not found

[AstroGD]

Sorry i was a bit misleading there, the error does not occurr on installation but on running code using knex after the installation

[ricardograca]

No idea how you're getting that error since that directory has a lowercase name on this repository and after installation. Can you either explain step by step how to reproduce this with a clean install or provide the necessary code to do so?

[AstroGD]

Another mistake from me: The Folder is named "mysql" and needs to be named "MySQL"

[AstroGD]

This is the error after a clean installation

[AstroGD]

Step by Step:

• Go into an empty folder
• type npm install knex
• Wait till finished
• put in any js file using knex
• run the js file
• get the error

[AstroGD]

Problem: When installing knex the folder is named "mysql" but it should be "MySQL"
Solution: Rename the folder to "MySQL" Path: node_modules/knex/lib/dialects

[ricardograca]

I'm using knex on a project right now with MySQL and not seeing that error. Can you provide the "any js file using knex" that you're using?

[AstroGD]

Ill provide it asap

[tgriesser]

The folder name is determined by whatever you provide as the config.client or config.dialect

https://github.com/tgriesser/knex/blob/2183a278268dc7aedf5edeccecb99176b9827f27/src/index.js#L29-L31

I'm guessing you have that as 'MySQL' instead of 'mysql', just fix that and it'll work.

[ricardograca]

@tgriesser Maybe coerce whatever the user provides to lower case since all the dialect directories are in lower case anyway.

[AstroGD]

Ah now I see. Thats what I did wrong.
I suggest to LowerCase everything so that this mistake does not happen again to others

[AstroGD]

Thanks for your help

[ricardograca]

I'm going to mark this as an enhancement since it's more about providing a better developer experience than an actual bug.

[elhigu]

Better to just throw an error about non-existing dialect than lowercasing automatically, which might cause still problems elsewhere.

[AstroGD]

Or maybe both? Try lowercasing it and throw an error if the problem is still not solved?

[elhigu]

Doing both is not any better than just lowercasing it. I would rather just force people to use correct parameter instead of trying to fix their input. IMO it would be also confusing that we are making some parts of knex caseinsensitive.

[ricardograca]

@elhigu I disagree. I think that Knex should be more helpful in this case, since it already does that in other similar cases.

The thing is that there is nothing stating that the client is actually a directory name of something provided by Knex, so I think it's only natural that some people will try to use the typical product name (e.g. MySQL).

Also there's no list of the valid client names on the documentation, how are people supposed to know they need to use only a specific string out of a restricted list for these?

Finally, there are already some aliases so that it's possible to type client names that are not exactly the actual directory name (e.g. pg and postgresql for postgres). So, Knex is already not forcing people to use correct parameters, so why stop there and not be even more tolerant to client name variations?

[elhigu]

Knex should be more helpful in this case, since it already does that in other similar cases

Does knex do that in other similar cases really (or maybe you were referring that alias list)? IMO having good error message and exception is more helpful that allowing case insensitive input in this case.

Also there's no list of the valid client names on the documentation

Yep. Valid names should be added to documentation.

Finally, there are already some aliases so that it's possible to type client names that are not exactly the actual directory name

I don't think that would has to be the directory name, but single correct name would have been nice to have in the first place. I have to agree that you have a point here :)

[kibertoad]

@elhigu I can create a PR that would validate the client param against constant set of known supported values and throw an explicit erro. Would that help?

[ricardograca]

Does knex do that in other similar cases really (or maybe you were referring that alias list)?

Isn't it the same thing? You can call MySQL and alias of mysql if you prefer 😉 Same thing with dateTime -> datetime, bigIncrements -> bigincrements. Seems like there are already a few cases where knex is indeed case insensitive.

I agree that having a more explicit error message is the preferred solution though.

[elhigu]

@kibertoad yep, I think that would be good solution. If you have spare time to add that, I'll gladly merge PR.

[kibertoad]

Sure, will work on it when I'm home.

[kibertoad]

Probably this issue can be closed now?