Support TypeScript exactOptionalPropertyTypes

[quisido]

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 FullStory.

Example:

function myInit({ devMode, orgId }) {
  init({ devMode, orgId });
}

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

In the above example, TypeScript 4.4 throws an error with the flag because devMode must either by a boolean 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. 🙃

[quisido]

I'm not sure if anyone is still maintaining this repo. Pinging @sabrina-li as the last committer.

[patrick-fs]

Hi @CharlesStover, apologies for the delayed reply. I reviewed this PR with a few other engineers and we think the types are cleaner as they are currently defined, without introducing a union type with undefined for optional parameters.

There are a few other options that you can pursue to implement the myInit() function in your example.

You could use a default parameter:

function myInit({ devMode = false, orgId }) {
  init({ devMode, orgId });
}

myInit({ orgId }); // will be valid

or you could avoid the issue by not restructuring and then reconstructing the options object provided to myInit() and just passing it directly into init(). Another option would be to use ?? to provide default values.

I'm going to close this PR given our perspective on the current type definitions and since there are alternatives to executing your desired example.

[quisido]

If the solution is to use default values, then can you provide the default values for all of FullStory's init parameters? The example here is contrived and minimal. My actual use case is to have a React component with the same prop types as FullStory's init.

will fail because the environment variable may not be defined, meaning the prop is not allowed to be specified if its value is undefined.

Not destructuring the props does not solve this.

Default values would solve this, but the default values for init are not known. Can you document them?

Alternatively, I still believe explicitly undefined values are still accurate.

init({ devMode: process.env.DEV_MODE }) should be considered type safe, but your library rejects it as invalid because the environment variable may not exist. In reality, it is safe and accurate for the environment variable not to exist. (Consider a string parameter instead of a boolean parameter.)

In summary, Can you either reconsider the typing or document the default values that should be used by any library needing to interact with this function?

[patrick-fs]

Hey @CharlesStover,

Again, apologies for the delayed reply. We have default values documented in the Readme, but another contributor recently added some doc strings that describe the purpose and default values of the snippet option properties: https://github.com/fullstorydev/fullstory-browser-sdk/blob/main/src/index.d.ts

Does this help you?

[quisido]

Thanks. I don't think this is the right approach versus | undefined, but it unblocks the use of init.

Now I can't pass displayName or email to identify if they are undefined. 🤦‍♂️ Any chance UserVars can be changed to displayName?: string | undefined, email?: string | undefined?