node-mssql icon indicating copy to clipboard operation
node-mssql copied to clipboard

Type hinting for `.execute` values from `.input` types

Open Doc999tor opened this issue 2 years ago • 5 comments

Type hinting is very helpful and reducing debugging time. In this case I'd very like if TS type hinting could point to obvious errors on the execute stage Example - in this case I removed the brackets from getId function to illustrate the problem

const query = `
	UPDATE users
	SET  name = @name
	    ,email = @email
	WHERE id = @user_id
`;
stmt.input('name', NVarChar);
stmt.input('email', VarChar);
stmt.input('user_id', Int); // user_id should be a number
await stmt.prepare(query);

const result: IResult<{ id: number }> = await stmt.execute({ 
	name, 
	email, 
	user_id: user.getId // I'd like to get a hint that user_id should be a number and not a function
});
await stmt.unprepare();

Or even simpler:

stmt.input('user_id', Int); // only user_id prepared statement
await stmt.prepare(`DELETE FROM users WHERE id = @user_id`);
await stmt.execute({ name, email, user_id: this.getId() }); // there is no .input for name and email

Expected behaviour:

stmt object already knows the user_id prop should be a number, but gets a function instead I'd expect the code to fall if execute will get the object with wrong value type The ideal case would be if TS could point the non-expected type before I run the code

Doc999tor avatar Nov 17 '21 18:11 Doc999tor

I think the typings probably need to be updated in the types library if you want that to work, no?

dhensby avatar Nov 17 '21 20:11 dhensby

Thanks for the quick reply

  1. If the lib itself could perform such checks (I mean to fall with detailed error/warning) - not only TS users will be thankful. I believe it can save hours of debugging and consolelogging - the SQL errors are rarely enough informative
  2. Since it's .d.ts files I guess it just cannot be done. I'll be glad to be wrong

Doc999tor avatar Nov 18 '21 01:11 Doc999tor

Even simpler example:

stmt.input('user_id', Int); // only user_id prepared statement
await stmt.prepare(`DELETE FROM users WHERE id = @user_id`);
await stmt.execute({ name, email, user_id: this.getId() }); // there is no .input for name and email

SQL returns cannot prepare the query-like very non-specific error It would be much helping if this life-saving lib could warn about inconsistency between registration (.input) and populating (.execute) of the arguments

Doc999tor avatar Nov 18 '21 03:11 Doc999tor

Well, there's no reason we couldn't warn against this kind of usage (it would be pretty bad practice to provide an object with more data than required for the execution), but in this kind of example we'd be forcing otherwise fine requests to fail.

In your latest example, this statement would execute fine, the problem would come when you didn't provide enough parameters for the defined inputs...

This feels a little bit too much like hand-holding and putting the developer on rails and also what unit tests are for (the errors you point out would be detected easily with some tests that asserted the application was correctly inserting to the database).

dhensby avatar Nov 18 '21 18:11 dhensby

😁 I'm the last person who wants to hand-hold developers - but I do want to do DX (dev experience) a bit better This feature could come before running unit tests - just to help to debug not always clear error codes of the SQL itself

Doc999tor avatar Nov 19 '21 03:11 Doc999tor