search

tediousjs / tedious

Node TDS module for connecting to SQL Server databases.
http://tediousjs.github.io/tedious/
MIT License
1.57k stars 436 forks source link

Authentication type parameter seems to accept too many values #1406

Open rzhao271 opened 2 years ago

rzhao271 commented 2 years ago

I noticed the allowed values for the authentication type parameter are being filtered down during runtime, but the type itself seems to accept string values, and even undefined.

I'm wondering whether the type of the authentication.type parameter can instead be set to something like 

type AuthenticationType = 'default' | 'ntlm' | 'azure-active-directory-password' | 'azure-active-directory-access-token' | 'azure-active-directory-msi-vm' | 'azure-active-directory-msi-app-service' | 'azure-active-directory-service-principal-secret'
// ...
type: AuthenticationType;

This way, users would know exactly which values are valid for a given version of tedious.

A screenshot showing the discrepancy

MichaelSun90 commented 2 years ago

Hi @rzhao271, I am wondering which version of tedious driver are you on? I just checked our latest code and seems we already handled it. You can check the latest source code at line 477 here. From there you can check the definition of the auth type and corresponding type handlings.

rzhao271 commented 2 years ago

Looks like I missed those lines. I'm using tedious with https://www.npmjs.com/package/@types/tedious because I noticed https://www.npmjs.com/package/tedious had a DT annotation.

MichaelSun90 commented 2 years ago

I do not think you have to use that since we do support our own types within tedious. You can give it a try, see if it meet your needs. Our project is here. Also, you can try to post this issue on their board, see if they can help you on this. Hi @arthurschreiber, I am not sure why there is a DT mark on the npm page for tedious, have we used their type definition before we fully migrate to typescript?

Fuco1 commented 1 year ago

When I install your project to a typescript project, there is no typing. The code installed from npm is javascript only, so the type declarations are missing.

MichaelSun90 commented 1 year ago

Hi @Fuco1, Thank you for bring this up. We are using babel to compile the typescript, and seems it does not handles the type declarations. For generate the declarations , tsc is needed. Hi @arthurschreiber , did we not generate it for a purpose or is just because of using babel. For type declarations, should we want to set something for tsc to generate it?