Fix #674 TypeScript issues with dynamic inserts

[alpharder]

Fixes #674

The problem was caused by the nature of how rest operator ... is treated in TypeScript, implying readonly (Keys & keyof R)[] to represent individual function arguments.

[readonly (Keys & keyof R)[]] represents exactly one argument of type readonly (Keys & keyof R)[]

[porsager]

Mind approving this @Minigugus & @karlhorky ?

[karlhorky]

@alpharder can you show a TS Playground or CodeSandbox/StackBlitz with your Postgres.js code where this fails? Would be easier to review what it is you're doing here.

[alpharder]

@karlhorky Sure: https://stackblitz.com/edit/stackblitz-starters-b2col8?file=index.ts

[karlhorky]

Thanks, from a code perspective that looks reasonable.

Do you have a documentation link for the sql(data, ["prop", "prop2"]) syntax? (and the sql(data, "prop", "prop2") syntax?). If so, that would be nice to link those two in the code comments as well.

My last question would be related to the ternary branches of the Rest generic type alias. I'm not super familiar with this type (maybe from @Minigugus ?) but would be maybe nice to write a line comment above each of these lines to describe what the difference is:

postgres/types/index.d.ts

Line 180 in ebc101d

T extends readonly (object & infer R)[] ? (

postgres/types/index.d.ts

Line 186 in ebc101d

T extends object ? (

[alpharder]

Do you have a documentation link for the sql(data, ["prop", "prop2"]) syntax? (and the sql(data, "prop", "prop2") syntax?). If so, that would be nice to link those two in the code comments as well.

I guess the only documentation available is the README.md file.
There was no other documentation links in the types/index.d.ts so I haven't felt the necessity to introduce any innovations.

I'm not super familiar with this type (maybe from @Minigugus ?)

T extends readonly (object & infer R)[] means the first argument is a list of objects with inferred type R of list elements
T extends object means the first argument is an object

but would be maybe nice to write a line comment above each of these lines to describe what the difference is:

I'm afraid that for consistency we'd have to cover the whole Rest type definition with lots of commentaries, while this seems to be either out of the scope for the current fix and redundant since it's pretty much self-explanatory

[karlhorky]

Ok, I'll leave these last decisions up to @porsager then, whether that's good enough like this - code looks ok, although a bit complex.

One last suggestion from my side: the suggested formatting of the union types in this PR is a bit unusual (see below)

  T extends readonly (object & infer R)[] ? (
    readonly (Keys & keyof R)[]   // sql(data, "prop", "prop2") syntax
    |
    [readonly (Keys & keyof R)[]] // sql(data, ["prop", "prop2"]) syntax
  ) :
  T extends readonly any[] ? readonly [] :
  T extends object ? (
    readonly (Keys & keyof T)[]   // sql(data, "prop", "prop2") syntax
    |
    [readonly (Keys & keyof T)[]] // sql(data, ["prop", "prop2"]) syntax
  ) :

Often unions are formatted without parentheses and by putting the pipe character at the start of each option in the union, eg. here's what Prettier does:

  : T extends readonly (object & infer R)[]
  ?
      | readonly (Keys & keyof R)[] // sql(data, "prop", "prop2") syntax
      | [readonly (Keys & keyof R)[]] // sql(data, ["prop", "prop2"]) syntax
  : T extends readonly any[]
  ? readonly []
  : T extends object
  ?
      | readonly (Keys & keyof T)[] // sql(data, "prop", "prop2") syntax
      | [readonly (Keys & keyof T)[]] // sql(data, ["prop", "prop2"]) syntax
  :

[alpharder]

@porsager Could you please merge this and issue a new release?

[porsager]

@karlhorky @alpharder is there anything considered breaking changes in this PR? Asked another way - Am I gonna have a horde of angry Typescripters at my door if it goes in a minor release? 😂

[alpharder]

Nope, they should be happy 😃

These changes are backwards-compatible