fix(types): Make name required on transaction class

[lobsterkatie]

Background

For a long time, the Transaction class and the Transaction interface have been incompatible, such that a member of the Transaction class could not be used where an object implementing the Transaction interface was expected. The sticking point is the name property, which is required in the interface but optional in the class. (Since name isn't guaranteed to be defined on a class instance, that instance can't sub in for something which requires name to be defined.)

We've been able to contort our way around this in the past by pointing to the less-restrictive Span interface, which doesn't include a name property at all. After all, transactions are spans, and so we can capture (most) of what it means to be a transaction by just referring to what it means to be a span.

But what about someplace where we actually care whether the thing we're pointing to is a full-blown transaction or merely a span? Then our workaround won't do, and we're stuck with the original problem.

Solution

One obvious answer here is to make name required on the Transaction class, because then it does implement the Transaction interface. But will that break anything? Two ways to know it won't:

• If we set a default value for the name property, then even if it's not provided, nothing goes wrong. And if that default value is the empty string (which is falsey), every if (transaction.name) construct will still have the same truth value as it would with an undefined name.

• There actually is currently no way to not provide the name property to the Transaction constructor, because it takes a TransactionContext argument as its first parameter, and name is required there.

The other solution, of course, would be to go the other way, and make name optional on the interface. The disadvantage there, though, is that then TS wouldn't harass people to provide a transaction name, which we actually very much want them to do. (The Discover UI is pretty bad if you don't name your transactions.) So if we can be sure that the first option is safe (which I claim it is), then it's clearly the preferable one.

[github-actions]

size-limit report

Path	Size
@sentry/browser - CDN Bundle (gzipped)	18.02 KB (0%)
@sentry/browser - Webpack	18.83 KB (0%)
@sentry/react - Webpack	18.83 KB (0%)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped)	23.89 KB (+0.02% 🔺)