TS4.4 exactOptionalPropertyTypes: options

[quisido]

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

• If you've added code that should be tested, please add tests.
• Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

I cannot lint in this instance, because I am PR'ing from the GitHub web UI. 🙃

---

TypeScript 4.4 added a new compiler option, exactOptionalPropertyTypes, which distinguishes between {} and { key: undefined }. With this flag, f(key?: string) may either be f() or f('str'), but may not be f(undefined). In most cases, f(undefined) should be acceptable, including in Sentry.

Example:

function myInit({ allowUrls }) {
  init({ allowUrls });
}

myInit({}); // should be valid,

In the above example, TypeScript 4.4 throws an error with the flag because allowUrls must either be an array or not set at all. It does not allow this code such that it is set to undefined.

For future consumer-facing code, please replace [key]?: T with [key]?: T | undefined. 🙃

---

Sorry in advance that each file is a separate PR. This also, however, allows blast radius to be reduced, and is particularly relevant for any properties where undefined should not be supported.

[kamilogorek]

Hey, thanks for the PR, and TIL about this flag from 4.4.

I do agree that in most cases it is indeed permissible in our codebase to pass undefined, however, I'm not sure whether once this flag becomes more widely used we should do this.
It has been introduced for a reason, to try and enforce better typing in the end. And because all of those options are user input, it would be nice if we could actually enforce that those options have either a valid type or not be provided at all.

Once we allow for passing undefined in TS4.4 and forward, it'll be basically impossible to go back without a major release. And I do understand that we do "allow" this now, however, it's only due to the fact that this flag did not exist previously, so anyone updating their TS version should be aware of the fact that it can break some typing. Especially that TS is not following SemVer.

I'll try to discuss this with the team soon, so I'll leave this PR without any changes for now.

Cheers!

[quisido]

I think the idea is you definitely do want to support undefined, with no going back. The idea of introducing this flag is to distinguish Object.prototype.hasOwnProperty.call(obj, 'id') on {} versus { I'd: undefined }, which has two different runtime behaviors. The same is the for Object.keys(obj), and it's why this flag matters.

If Sentry is not relying on the keys existing, you would want the explicit use of undefined here so that both styles pass transpilation, since both styles produce the same runtime behavior.

For an example of how one must interact with Sentry with optional properties, and why I opened this PR, see my code here:
https://github.com/CharlesStover/charlesstover.com/blob/main/src/modules/react-sentry/src/components/sentry/sentry.view.root.hook.ts#L111

The whole checks and optional setting could be so much simpler as just { a, b, c }, but the typing is telling me the key cannot exist if the value is undefined. Since the runtime behavior is the same whether undefined or unset, I feel this should be changed.

[kamilogorek]

@CharlesStover I cherry-picked your commits and created a joined PR + updated NodeOptions here #3960
As mentioned, will discuss it with the team this week. Cheers!