1953: Avoid non-null assertions for WhoAmI

[steffenkleinle]

Short Description

Avoid non-null assertions for who am i by creating a hook.

Proposed Changes

• Create a hook to expose the context non-nullable

Side Effects

• Using the hook is only possible if me is actually not null, so when a user is logged in

Testing

Ensure that everything still works.

Resolved Issues

Fixes: #1953

[michael-markl]

I think we should throw an error, if me is null.
Also, I think it's not necessary to create a new object, we can just return whatever useContext(WhoAmIContext) returns.

I am also a bit confused about the @ts-expect-errror on line 58. 🤔

[steffenkleinle]

I think we should throw an error, if me is null.

👍

Also, I think it's not necessary to create a new object, we can just return whatever useContext(WhoAmIContext) returns.

I am also a bit confused about the @ts-expect-errror on line 58. 🤔

Typescript seems to have problems when checking object properties for nullish values and raises an error in that case. This is why we have the @ts-expect-error. Same for directly returning the context without destructuring/creating a new object like this:

  const context = useContext(WhoAmIContext)
  if (context.me === null) {
    throw new Error('')
  }
  // TS 2322: Type null is not assignable to type ...
  return context

[michael-markl]

Yeah, this seems to be a known limitation of typescript 🤔 . Probably, it's better to do

  const context = useContext(WhoAmIContext)
  if (context.me === null) {
    throw new Error('WhoAmI context is not available')
  }
  return context as UseWhoAmIReturn

(and below we can probably do value as WhoAmIContextType instead of ignoring the error)

[steffenkleinle]

Hmm, I really don't like typecasts if we can avoid them, do you think that is a better solution here instead of destructuring the object in this case? I agree for below, doesn't really matter if we have a @ts-expect-error or a typecast, but looks cleaner.

[michael-markl]

You could also abstract away this typecast in a type guard like

const hasProp= <
  P extends PropertyKey,
  O extends { [p in P]: unknown }
>(obj: O, p: P): obj is (O & { [p in P]: NonNullable<unknown> }) => obj[p] !== undefined && obj[p] !== null


export const useWhoAmI = (): UseWhoAmIReturn => {
  const context = useContext(WhoAmIContext)
  if (!hasProp(context, 'me')) {
    throw Error("WhoAmI context is not available")
  }
  return context
}

Maybe, that's the cleanest, but slightly overwhelming solution...