Treat undefined as an error

[JamesApple]

With Typescript 4.4 having optional property types I would love to see undefined in wheres/updates/inserts such as the following made either a type or runtime error.

sql`SELECT * FROM users WHERE {{ userId: undefined }}`
// OR
sql`SELECT * FROM users WHERE {{ userId: user?.userId }}`

It's obvious to the reader that userId could never be null but we still fallback to the node-postgres default of treating undefined as null which results in a query that potentially cannot be executed or has unexpected side effects. Personally I've found this to be my only pain point thusfar but I think it's a biggie and I ran headfirst into it as I didn't see it mentioned in the documentation.

I think if no type changes are possible then perhaps Zapatos could have a global option on how to treat explicit undefined values. Maybe throw, remove, or treat as null?

I've had to wrap the query methods available in my app to filter undefined values as a solution in the meantime but I'd hate to have to do that for every app I write.

Big fan of the library anyway and I really appreciate the effort you've put into keeping this library so focused and minimal.

[jawj]

Huh, interesting. This issue has come up before: #46 (comment). But of course we couldn't distinguish undefined from missing then.

I'll try to take a look at this soon.

[TheNickmaster21]

I know this is being discussed in the context of SELECT statements but this has become a recurring annoyance for my team when dealing with INSERT statements. Consider the following with table 'bools' with a single field 'value' with NOT NULL constraint and a default value of FALSE:

db.insert('bools', { value: undefined });

The above statement compiles without a problem even though it is guaranteed to throw a runtime exception. If instead fields with a value of undefined were treated as undefined, value could be excluded from the generated INSERT statement and this error would be avoided.

An alternative solution that my team would find useful would be for the Insertable interface to not include 'undefined' as a possible type for a field with a default value while still having the field be optional. Any way that would remove our need to check for explicitly undefined values on objects we use with the insert function would be great.

[jawj]

Thanks. I can see this is a pain. I haven't had a lot of time lately to work on the library, but I'll see if I can have a bit of a think about this soon.

[jawj]

@TheNickmaster21 Have you checked what happens when you switch on exactOptionalPropertyTypes in your tsconfig.json file?

From a brief look, this appears to do what's needed without any changes to Zapatos: it turns your example into a type error, but doesn't appear to break anything else (not in a couple of my projects, at least).

[jawj]

Oh, but hang on: does this address your issue, or are you saying that you'd like it to remain not-a-type-error, but to be treated by the library the same as if the key had been omitted?

[jawj]

@JamesApple Have you tried exactOptionalPropertyTypes, and is this working how you wanted at the top of the thread? (I haven't actually changed anything, but it seems to do the right thing for me anyway).

[TheNickmaster21]

We would prefer if the library treated it the same as if the key was omitted. We are currently running TypeScript 4.3.5 so we can't enable that flag. I can see if it's possible for us to update our TypeScript version but I will have to update our CI/CD pipeline and some other tooling. Thanks for the consideration either way.

EDIT: I was able to quickly update TS versions locally to test the flag and it causes a lot of type issues across our project. So that solution, at least in the short term, is not viable for our team.

[jawj]

@TheNickmaster21 I guess an appropriately typed utility function that removes keys with undefined values might be a useful tool in your case? For example:

function defined<T extends Record<string | number | symbol, any>>(obj: T) {
  return Object.fromEntries(Object.entries(obj).filter(([, v]) => v !== undefined)) as
    { [k in keyof T as T[k] extends undefined ? never : k]: T[k] };
}

Something like this then works correctly and is properly type-checked:

await db.select('authors', defined({ name: undefined })).run(pool);

(The Whereable here becomes an empty object, which means no WHERE conditions: all records are returned).

[TheNickmaster21]

We've actually already added such a function for ourselves. It's just inconvenient to have to remember to use the function or possibly run into run time errors.

I suppose our primary complaint here is that function is not ignoring the undefined values. I know the library choses to treat undefined values as null to mimic/carry over the behavior of pg but in the case where a utility function is being used (db.select for example), we were hoping the behavior would be different.

I do think it's fair that the best solution here is to update our own application to the latest TypeScript and fix the places we are not compliant with exactOptionalPropertyTypes, we just won't be able to any time soon.

[jawj]

Great. Sorry it's a pain for you, but yes, I agree — if and when you can turn on exactOptionalPropertyTypes, you won't have to remember to use the utility function and you won't get runtime errors, because the type system should enforce it.

I'll hold off closing this for a few days in case @JamesApple (who opened the issue) has any further comments ...