ref(tracing): Make tracing integrations tree shakeable

[AbhiPrasad]

Supercedes #4199 and #4200

Fixes #3310 properly.

Cuts ~2kb for nextjs client gzip + minified bundle.

This works because we avoid the object spread in the index, we instead just take advantage of js exporting the object correctly. See an example in the rollup repl. For more info, see the details at the end of the PR description.

RIP Mongo in the browser tracing bundle!

Before

After

Details

We previously exported integrations in tracing like so:

import * as TracingIntegrations from './integrations.js';
import { BrowserTracing } from './browsertracing.js';
const Integrations = { ...TracingIntegrations, BrowserTracing };

export { Integrations };

because we created a new object and then exported it, the classes would always get included:

class Mongo {}

class Postgres {}

var TracingIntegrations = /*#__PURE__*/Object.freeze({
	__proto__: null,
	Mongo: Mongo,
	Postgres: Postgres
});

class BrowserTracing {}

const Integrations = { ...TracingIntegrations, BrowserTracing };

Webpack/Rollup will always include the TracingIntegrations object, so this means it would always pull in the classes.

We changed this like so

// int.js
export * from './integrations.js';
export { BrowserTracing } from './browsertracing.js';

import * as Integrations from './int.js'
export { BrowserTracing } from './browsertracing.js';

export { Integrations };

This just makes each integration part of the module exports, so webpack/rollup can tree shake them out effectively. This way, a user can import BrowserTracing without bringing in the rest of the modules.

Next Steps

Update docs after release so that users get this for free. They'll have to directly import BrowserTracing instead of doing Integrations.BrowserTracing.

[github-actions]

size-limit report

Path	Base Size (d17f049)	Current Size	Change
@sentry/browser - CDN Bundle (gzipped)	22.45 KB	22.45 KB	+0.01% 🔺
@sentry/browser - Webpack	23.29 KB	23.29 KB	0%
@sentry/react - Webpack	23.32 KB	23.32 KB	0%
@sentry/nextjs Client - Webpack	49.7 KB	47.97 KB	-3.48% 🔽
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped)	29.89 KB	29.9 KB	+0.01% 🔺

[lobsterkatie]

Good change! All but one of my comments are just about making it clearer to the reader what's going on and why.

On that topic:

This works because we avoid the object spread in the index, we instead just take advantage of js exporting the object correctly. See an example in the rollup repl.

Two questions here:

1. I'm not clear what you mean by "avoid object spread in the index," or what it means for JS to export the object correctly.

2. (potentially solved by solving part 1 above) What is the repl illustrating, and how? Maybe have the bad way commented out so people can switch back and forth to see the effect?

[lobsterkatie]

In both its old and new versions, I find this test confusing. How does the fact that every value in the Integrations object is itself an object with a string-valued id property prove anything about how things are getting exported? (This applies to the version in index.test.ts also.)

(Also, what's the advantage of the refactor here?)

[AbhiPrasad]

I just made it so that we don't need to use testOnlyIfNodeVersionAtLeast anymore.

[AbhiPrasad]

Yeah good question on the value of this test. I just copied the existing one from index.bundle.test.ts into index.test.ts, but open to alternatives.

[lobsterkatie]

I just made it so that we don't need to use testOnlyIfNodeVersionAtLeast anymore.

I figured that might be the case. My thought about that, though, is that if you leave aside the node restriction, IMHO the old test is simpler, because you don't need the keyof typeof stuff, and since we're about to drop node 6 compatibility anyway, we could just remove the wrapper when we do and still get to have a simpler test. What do you think?

That said, yeah, first we have to figure out if we even want the test in the first place! My issue with it is less "is there value in testing the exportation?" (though I'm iffy on that) and more "okay, but how does any of this prove that it's exported correctly?". Do you understand the logic there?

[AbhiPrasad]

whether to deal with it here or come back to it

I will come back to this and address this in another PR. You make great points that we have to figure out.

[lobsterkatie]

Nice writeup! 💯

LGTM

(As for the test we're discussing above, I don't see how the logic holds water, but I'll leave the ball in your court to decide whether to deal with it here or come back to it. Happy to rubber duck if you want to talk through whether it does or doesn't make any sense.)