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

Type hinting for .execute values from .input types #1336

Open
Doc999tor opened this issue Nov 17, 2021 · 5 comments
Open

Type hinting for .execute values from .input types #1336

Doc999tor opened this issue Nov 17, 2021 · 5 comments

Comments

@Doc999tor
Copy link

Doc999tor commented Nov 17, 2021

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

@dhensby
Copy link
Collaborator

dhensby commented Nov 17, 2021

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

@Doc999tor
Copy link
Author

Doc999tor commented Nov 18, 2021

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
Copy link
Author

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

@dhensby
Copy link
Collaborator

dhensby commented Nov 18, 2021

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).

@Doc999tor
Copy link
Author

😁
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants