Incorrect types for loaders that return optional properties

[JNaftali]

What version of Remix are you using?

v1.6.5

Steps to Reproduce

Simple reproduction in the Typescript playground

1. return an object from a loader with keys that may be optional/undefined
2. attempt to infer the type of the object using useLoaderData<typeof loader>
3. typescript can be made to think that a variable containing undefined is of type string rather than string | undefined

Expected Behavior

optional properties should be considered potentially undefined when returned from useLoaderData - even though JSON.parse will never include an undefined value, it's possible that expected keys will simply be missing which (in JS) is almost equivalent to being actually undefined).

Actual Behavior

any undefineds or optional properties are stripped away entirely.

[silvenon]

As JSON doesn't support undefined, the correct type of data should probably be {} | { example: string }. At least according to the object that useLoaderData() returns.

[tshddx]

As JSON doesn't support undefined, the correct type of data should probably be {} | { example: string }. At least according to the object that useLoaderData() returns.

That's technically true, but I think { example?: string } in the return type of useLoaderData should be fine. Technically there are some runtime differences, like after JSON.parse it would be impossible for 'example' in data && data.example === undefined to be true, but TypeScript doesn't appear to do any sort of type guarding around the in operator, so I don't think you could end up with either a false positive TS error or a JS runtime error that TS didn't catch.

[JNaftali]

the current type behavior encourages runtime errors (by telling Typescript that a key is definitely there when in fact it might not be). My proposed change might not be perfect, but it's miles better than that. If someone else proposes a better fix I am far from attached to my solution, but it is a solution (and a simple one at that)

[silvenon]

TypeScript doesn't appear to do any sort of type guarding around the in operator

@tshddx not sure what you mean, if data is of type {} | { example: string } then TypeScript will allow you to narrow the type down using in:

data.example // error because `example` key might not exist
if ("example" in data) {
  // here you can access data.example
}

I wouldn't know how to submit that PR in Remix, though, so I'll be quiet. 😄

[JNaftali]

if data is of type {} | { example: string } then TypeScript will allow you to narrow the type down using in:

TS handles { thing?: string } and { thing: string | undefined } identically with respect to the in operator, even though strictly speaking you'd expect 'thing' in whatever to be either true or false for the first one - in neither case is it considered enough for a type guard to tell you whether thing is string or undefined https://tsplay.dev/wOGqzw

[silvenon]

This part of TypeScript is tricky, yes, I'm still not entirely used to it, however in does appear to achieve the desired effect! If you correct the two mistakes in your example:

1. example.foo.split() instead of example.split()
2. pass an argument to .split()

No type errors! For optional properties both if ("foo" in example) and if (example.foo) seem to work, but for {} | { foo: string }, which is what JSON.parse(JSON.stringify({ foo: getStringOrUndefined() })) would return (in theory), only in would work.

My conclusion: Having the type return from useLoaderData be string | undefined would definitely be much better than the bug that exists now. 👍

[JNaftali]

Fixed by #3766