ref(utils): Use type predicates in is utility functions

[lobsterkatie]

We have a number of is utility functions (isString, isError, and so on), and while they tell the human reading the code something about the value in question, they don't tell TypeScript anything, because it doesn't see them as type checks. As a result, we end up with a lot of code like this:

if( isString(msg) ) {
  const words = (msg as string).split(' ');
}

where we have to typecast our variables, despite their type already having been checked immediately beforehand.

In order to fix this, TS lets you return a type predicate from a type-checking function rather than a simple boolean, and that lets it know what the human reader knows - if isString(msg) is true, then msg must be a string.

This switches all of our is functions to use type predicates, and removes all of the typecasts which are no longer necessary as a result of this change.

[github-actions]

size-limit report

Path	Size
@sentry/browser - CDN Bundle (gzipped)	22.45 KB (-0.12% 🔽)
@sentry/browser - Webpack	23.35 KB (-0.03% 🔽)
@sentry/react - Webpack	23.38 KB (-0.03% 🔽)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped)	29.89 KB (-0.1% 🔽)

[AbhiPrasad]

Nice, good refactor

[AbhiPrasad]

we can even enforce this with an eslint rule, what do you think?

[lobsterkatie]

we can even enforce this with an eslint rule, what do you think?

What exactly would you test for?

[AbhiPrasad]

That isX functions in is.ts use type predicates instead of boolean returns.

maybe it’s not worth the hassle though.

[lobsterkatie]

That isX functions in is.ts use type predicates instead of boolean returns.

maybe it’s not worth the hassle though.

Ah. Didn't know if you meant that or checking for unnecessary casts. In any case, yeah, I don't think checking the functions themselves is worth it - we add new ones so infrequently, and whoever does the next one will presumably see that all of the others use predicates and will hopefully follow suit.