Prevent bind error with undefined/null parameters #375
Conversation
lovasoa
left a comment
There was a problem hiding this comment.
values has type Statement.BindParams, which is defined as Database.SqlValue[]|Object<string, Database.SqlValue> so it cannot be undefined or null. If a library user passes undefined or null, it is an error on their side.
We can discuss changing the type Statement.BindParams, but there should be a good reason for changing the API of the library, and the type definitions will also need to be updated, not just the code.
|
The code you are pointing to (https://github.com/typeorm/typeorm/blob/master/src/driver/sqljs/SqljsQueryRunner.ts#L44-L55) is clearly a bug in typeorm. |
|
Thank you for your response, i'm agree with you but the typeorm code works (in my case) with sql.js version 0.5.0...Is there any documentation about migration or breaking changes in api between 0.X and 1.X ? |
|
@fb64 OK, I've run a little investigation, and you're right ! The switch from coffeescript to javascript is the origin of this problem. EDIT So, the fact that the code worked for null and undefined was kind of accidental, but it looks like dependent libraries came to rely on it. So I think we should document the behavior and reimplement it. |
|
Thank you for your investigation @lovasoa . I'll prepare a PR on typeorm to fix this. |
|
To be clear: I'm ready to merge this PR. We just need to :
@fb64 are you ready to do this? |
Yes, I'll try. |
In some client library (eg: typeorm) Statement.bind function could be called with undefined or null parameters. https://github.com/typeorm/typeorm/blob/ce07824df4305cb93222959db0804a7d62218c09/src/driver/sqljs/SqljsQueryRunner.ts#L55
…bind() with null as parameter.
lovasoa
left a comment
There was a problem hiding this comment.
Thank you, this looks good ! I'll just like to change the type check, and we'll merge :)
|
Thank you very much @fb64 for your contribution ! |
In some client library (eg: typeorm) Statement.bind function could be called with undefined or null parameters.
https://github.com/typeorm/typeorm/blob/ce07824df4305cb93222959db0804a7d62218c09/src/driver/sqljs/SqljsQueryRunner.ts#L55