feat(v8/integrations): Merge integrations into core by s1gr1d · Pull Request #10799 · getsentry/sentry-javascript

s1gr1d · 2024-02-23T13:31:09Z

Move relevant integrations. Part of #9833

AbhiPrasad

Given HttpClient and ReportingObserver integrations are going straight into @sentry/browser, how about we move all bundling logic from core to browser package?

So packages/core has no bundle build (and no rollup.bundle.config.mjs), and instead we just add new bundles to browser workflow.

This means that with the removal of @sentry/integrations we consolidate all the CDN building logic to one central package.

Lms24

I agree, creating all integration CDN bundles from browser seems like a good idea to me too.

I suggest we do this in a couple of steps:

move "universal" integrations into core in this PR (what you already did in this PR minus the CDN bundle changes)
move browser-only integrations into browser (I think you already got CDN bundles working for them so feel free to include them in the PR or in a separate one, whatever you prefer)
add missing integration CDN bundles in browser package

github-actions · 2024-02-26T13:49:36Z

size-limit report 📦

Path	Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped)	77.27 KB (-0.02% 🔽)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)	68.54 KB (-0.02% 🔽)
@sentry/browser (incl. Tracing, Replay with Canvas) - Webpack (gzipped)	72.45 KB (-0.02% 🔽)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped)	62.08 KB (+0.04% 🔺)
@sentry/browser (incl. Tracing) - Webpack (gzipped)	32.7 KB (-0.08% 🔽)
@sentry/browser (incl. browserTracingIntegration) - Webpack (gzipped)	32.7 KB (-0.08% 🔽)
@sentry/browser (incl. Feedback) - Webpack (gzipped)	30.91 KB (-0.02% 🔽)
@sentry/browser (incl. sendFeedback) - Webpack (gzipped)	30.91 KB (-0.02% 🔽)
@sentry/browser - Webpack (gzipped)	22.18 KB (-0.08% 🔽)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped)	75.69 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)	67.39 KB (-0.1% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)	33.17 KB (-0.2% 🔽)
@sentry/browser - ES6 CDN Bundle (gzipped)	24.72 KB (-0.06% 🔽)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed)	211 KB (-0.02% 🔽)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed)	99.72 KB (-0.05% 🔽)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed)	73.92 KB (-0.06% 🔽)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped)	36.26 KB (-0.07% 🔽)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped)	68.79 KB (-0.06% 🔽)
@sentry/react - Webpack (gzipped)	22.19 KB (-0.12% 🔽)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped)	85.23 KB (-0.02% 🔽)
@sentry/nextjs Client - Webpack (gzipped)	49.57 KB (-0.12% 🔽)
@sentry-internal/feedback - Webpack (gzipped)	17.14 KB (0%)

mydea · 2024-02-27T14:09:15Z

dev-packages/browser-integration-tests/utils/generatePlugin.ts

  return Object.fromEntries(
    packageNames.map(packageName => {
+      // pluggable integrations exist in the browser package
+      const actPackageName = packageName === 'integrations' ? 'browser' : packageName;


I think this can be removed again - there is no more package with that name I believe?

mydea · 2024-02-27T14:10:00Z

dev-packages/rollup-utils/bundleHelpers.mjs

  };

-  // used by `@sentry/integrations` and `@sentry/wasm` (bundles which need to be combined with a stand-alone SDK bundle)
+  // used by `@sentry/wasm` (bundles which need to be combined with a stand-alone SDK bundle)


Suggested change

// used by `@sentry/wasm` (bundles which need to be combined with a stand-alone SDK bundle)

// used by `@sentry/wasm` & pluggable integrations from core/browser (bundles which need to be combined with a stand-alone SDK bundle)

this is a bit more accurate I guess?

mydea · 2024-02-27T14:10:44Z

packages/browser/rollup.bundle.config.mjs

  throw new Error('JS_VERSION must be either "es5" or "es6"');
 }

+const addonIntegrationFiles = ['contextlines', 'httpclient', 'reportingobserver'];


let's call this browserPluggableIntegrationFiles maybe? A bit more explicit what it does, wdyt?

Sounds good, fits better with coreIntegrationFiles below 👍🏻

mydea

Great work! 🚀

Lms24

Awesome!

Lms24 · 2024-02-27T15:18:27Z

MIGRATION.md

+
+We moved optional integrations from their own package (`@sentry/integrations`) to `@sentry/browser` and `@sentry/node`.
+
+Integrations that are now exported from `@sentry/browser` :


l: just for clarity

Suggested change

Integrations that are now exported from `@sentry/browser` :

Integrations that are now exported from `@sentry/browser` (or framework-specific packages like `@sentry/react`):

Lms24 · 2024-02-27T15:22:42Z

dev-packages/rollup-utils/bundleHelpers.mjs

  };

-  // used by `@sentry/integrations` and `@sentry/wasm` (bundles which need to be combined with a stand-alone SDK bundle)
+  // used by `@sentry/wasm` & pluggable integrations from core/browser (bundles which need to be combined with a stand-alone SDK bundle)


(not relevant for this PR): This reminds me, we should also move @sentry/wasm into browser. It's also just an integration. So now that we can build and publish integration CDN bundles in browser, I think it should be fairly straight forward to bring it in.

Lms24 · 2024-02-27T15:24:08Z

packages/browser/rollup.bundle.config.mjs


+const browserPluggableIntegrationFiles = ['contextlines', 'httpclient', 'reportingobserver'];
+
+const coreIntegrationFiles = ['captureconsole', 'debug', 'dedupe', 'extraerrordata', 'rewriteframes', 'sessiontiming'];


super-l: wdyt about naming this corePluggableIntegrationFiles for consistency?

s1gr1d requested review from Lms24 and lforst February 23, 2024 13:31

s1gr1d force-pushed the sig-merge-integrations-to-core branch from d05c14e to d65055e Compare February 23, 2024 13:40

AbhiPrasad reviewed Feb 23, 2024

View reviewed changes

Lms24 reviewed Feb 23, 2024

View reviewed changes

s1gr1d force-pushed the sig-merge-integrations-to-core branch from aeb4922 to e3e14ba Compare February 26, 2024 12:34

s1gr1d requested review from AbhiPrasad and Lms24 February 26, 2024 12:35

s1gr1d self-assigned this Feb 26, 2024

s1gr1d force-pushed the sig-merge-integrations-to-core branch from 0612a03 to b321096 Compare February 27, 2024 13:10

mydea reviewed Feb 27, 2024

View reviewed changes

s1gr1d force-pushed the sig-merge-integrations-to-core branch from b321096 to ec37e37 Compare February 27, 2024 15:13

mydea approved these changes Feb 27, 2024

View reviewed changes

Lms24 approved these changes Feb 27, 2024

View reviewed changes

s1gr1d added 11 commits February 27, 2024 17:01

merge integrations into core

f14e393

revert bundle changes in core

ec635da

export browser integrations from core

b5e7ed1

delete integrations package

4d1923a

delete sentry/integration deps

94af457

fix formatting

8c8b98c

export integrations from node package

d1236f6

fix test errors

487278d

fix missing dedupe integration

9e34606

review comments

82bfc58

more review comments

108f53f

s1gr1d force-pushed the sig-merge-integrations-to-core branch from d171f49 to 108f53f Compare February 27, 2024 16:01

s1gr1d enabled auto-merge (squash) February 27, 2024 16:01

s1gr1d merged commit a5c4fcb into develop Feb 27, 2024

s1gr1d deleted the sig-merge-integrations-to-core branch February 27, 2024 16:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(v8/integrations): Merge integrations into core#10799

feat(v8/integrations): Merge integrations into core#10799
s1gr1d merged 11 commits intodevelopfrom
sig-merge-integrations-to-core

s1gr1d commented Feb 23, 2024 •

edited

Loading

Uh oh!

AbhiPrasad left a comment

Uh oh!

Lms24 left a comment

Uh oh!

github-actions bot commented Feb 26, 2024 •

edited

Loading

Uh oh!

mydea Feb 27, 2024

Uh oh!

mydea Feb 27, 2024

Uh oh!

mydea Feb 27, 2024

Uh oh!

s1gr1d Feb 27, 2024

Uh oh!

mydea left a comment

Uh oh!

Lms24 left a comment

Uh oh!

Lms24 Feb 27, 2024

Uh oh!

Lms24 Feb 27, 2024

Uh oh!

Lms24 Feb 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	// used by `@sentry/wasm` (bundles which need to be combined with a stand-alone SDK bundle)
	// used by `@sentry/wasm` & pluggable integrations from core/browser (bundles which need to be combined with a stand-alone SDK bundle)


		We moved optional integrations from their own package (`@sentry/integrations`) to `@sentry/browser` and `@sentry/node`.

		Integrations that are now exported from `@sentry/browser` :


		const browserPluggableIntegrationFiles = ['contextlines', 'httpclient', 'reportingobserver'];

		const coreIntegrationFiles = ['captureconsole', 'debug', 'dedupe', 'extraerrordata', 'rewriteframes', 'sessiontiming'];

Uh oh!

Conversation

s1gr1d commented Feb 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AbhiPrasad left a comment

Choose a reason for hiding this comment

Uh oh!

Lms24 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

mydea Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

mydea Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

mydea Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

s1gr1d Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

mydea left a comment

Choose a reason for hiding this comment

Uh oh!

Lms24 left a comment

Choose a reason for hiding this comment

Uh oh!

Lms24 Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

Lms24 Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

Lms24 Feb 27, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

s1gr1d commented Feb 23, 2024 •

edited

Loading

github-actions bot commented Feb 26, 2024 •

edited

Loading