Prevent bind error with undefined/null parameters

[fb64]

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

[lovasoa]

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.

[lovasoa]

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.

[fb64]

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 ?
Also I'll try to see with typeorm team to fix that on their side...

[lovasoa]

@fb64 OK, I've run a little investigation, and you're right !

The switch from coffeescript to javascript is the origin of this problem.
In bindFromObject, we had code that translated to a for..in loop, which works for null and undefined, and we ported it to Object.keys(...).forEach, which doesnt.

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.

[fb64]

Thank you for your investigation @lovasoa . I'll prepare a PR on typeorm to fix this.

[lovasoa]

To be clear: I'm ready to merge this PR. We just need to :

• update the typedef for BindParams,
• specify the behavior of bind when given null in its doc comment, and
• add a test for this

@fb64 are you ready to do this?

[fb64]

To be clear: I'm ready to merge this PR. We just need to :

• update the typedef for BindParams,
• specify the behavior of bind when given null in its doc comment, and
• add a test for this

@fb64 are you ready to do this?

Yes, I'll try.

[lovasoa]

Thank you, this looks good ! I'll just like to change the type check, and we'll merge :)

[lovasoa]

Thank you very much @fb64 for your contribution !