Conflicting eslint rules re: TypeScript non-null assertions

[lindapaiste]

I decide to work on #494 and I've come across a case of conflicting eslint rules. Of course I could eslint-disable-next-line, but I think it would be better for this project to have a standard of how it handles type assertions.

The problematic function is this:

function onDone(error: Error | null, result: T | null) {
  finished = true
  clearTimeout(overallTimeoutTimer)

  if (!usingJestFakeTimers) {
    clearInterval(intervalId)
    observer.disconnect()
  }

  if (error) {
    reject(error)
  } else {
    resolve(result!)
  }
}

I initially wrote resolve(result as T), but this caused an error

122:13 error Use a ! assertion to more succintly remove null and undefined from the type @typescript-eslint/non-nullable-type-assertion-style

So I used the ! as recommended and wrote resolve(result!), but then I got an error telling me not to

122:13 error Forbidden non-null assertion @typescript-eslint/no-non-null-assertion

The reason that this needs an assertion is the questionable assumption that if error is null then result must be a value. When looking at this function in isolation it's not a safe assumption because error and result are two unrelated variables. In the context of the file you can see that one is always null, so then it's ok to assert.

Of course there are ways to restructure the code to avoid the need for the assertion, but it seems like that is not desired based on this comment. This is a pretty simple assertion, I'm just unsure of which syntax is preferred.

[eps1lon]

Thanks for the work!

You can use @typescript-eslint/no-non-null-assertion and add a comment why the non-null assertion is correct. TypeScript is unaware of some assumptions so we have to provide these to explain to the compiler what's happening.