palantir / conjure-typescript

Conjure generator for TypeScript clients
Apache License 2.0
17 stars 16 forks source link

Have a single way to represent absent values: use undefined #156

Open Que3216 opened 3 years ago

Que3216 commented 3 years ago

The typescript coding guidelines state:

Use undefined. Do not use null.

https://github.com/Microsoft/TypeScript/wiki/Coding-guidelines#null-and-undefined

Standardizing on one helps prevent bugs (due to comparisons between the two; or due to === undefined checks, when the value is really null). Using the two to represent different semantics usually leads to more confusing code. In addition since Java can't distinguish between the two any undefineds are converted to nulls after a write and a read.

Should we therefore:

  1. Update all Conjure generated typescript typings to be '| undefined' only (instead of | undefined | null)?
  2. Update the generated Conjure clients to automatically convert any nulls to undefined on deserialization?

This would be an API break, so would have to be opt in --- ideally opt-in on the client side (maybe we have Conjure publish two versions of the typescript typings, a v1 and a v2?). The goal would be to reduce bugs from null vs undefined mismatches, and just have a single way of representing optional values.

p-szm commented 3 years ago

I just wanted to flag that the coding guidelines that you linked to are only for contributing to TypeScript itself, and are not general TypeScript coding guidelines. See this warning at the top of the page:

These are Coding Guidelines for Contributors to TypeScript. This is NOT a prescriptive guideline for the TypeScript > community. These guidelines are meant for contributors to the TypeScript project's codebase. We have chosen many of them > for team consistency. Feel free to adopt them for your own team.

AGAIN: This is NOT a prescriptive guideline for the TypeScript community