Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(feedback): Create async bundles and code to resolve helper integrations #11621

Merged
merged 16 commits into from
Apr 21, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions .size-limit.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,35 +52,28 @@ module.exports = [
path: 'packages/browser/build/npm/esm/index.js',
import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'feedbackIntegration'),
gzip: true,
limit: '83 KB',
limit: '86 KB',
},
{
name: '@sentry/browser (incl. Feedback)',
path: 'packages/browser/build/npm/esm/index.js',
import: createImport('init', 'feedbackIntegration'),
gzip: true,
limit: '37 KB',
},
{
name: '@sentry/browser (incl. Feedback, Feedback Modal)',
path: 'packages/browser/build/npm/esm/index.js',
import: createImport('init', 'feedbackIntegration', 'feedbackModalIntegration'),
gzip: true,
limit: '37 KB',
limit: '40 KB',
},
{
name: '@sentry/browser (incl. Feedback, Feedback Modal, Feedback Screenshot)',
name: '@sentry/browser (incl. sendFeedback)',
path: 'packages/browser/build/npm/esm/index.js',
import: createImport('init', 'feedbackIntegration', 'feedbackModalIntegration', 'feedbackScreenshotIntegration'),
import: createImport('init', 'sendFeedback'),
gzip: true,
limit: '40 KB',
limit: '28 KB',
},
{
name: '@sentry/browser (incl. sendFeedback)',
name: '@sentry/browser (incl. FeedbackAsync)',
path: 'packages/browser/build/npm/esm/index.js',
import: createImport('init', 'sendFeedback'),
import: createImport('init', 'feedbackAsyncIntegration'),
gzip: true,
limit: '30 KB',
limit: '33 KB',
},
// React SDK (ESM)
{
Expand Down Expand Up @@ -167,6 +160,13 @@ module.exports = [
brotli: false,
limit: '220 KB',
},
{
name: 'CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed',
path: createCDNPath('bundle.tracing.replay.feedback.min.js'),
gzip: false,
brotli: false,
limit: '261 KB',
},
// Next.js SDK (ESM)
{
name: '@sentry/nextjs (client)',
Expand Down
13 changes: 13 additions & 0 deletions packages/browser/src/feedback.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import {
buildFeedbackIntegration,
feedbackModalIntegration,
feedbackScreenshotIntegration,
} from '@sentry-internal/feedback';
import { lazyLoadIntegration } from './utils/lazyLoadIntegration';

// The full feedback widget, with everything pre-loaded
export const feedbackIntegration = buildFeedbackIntegration({
lazyLoadIntegration,
getModalIntegration: () => feedbackModalIntegration,
getScreenshotIntegration: () => feedbackScreenshotIntegration,
});
7 changes: 7 additions & 0 deletions packages/browser/src/feedbackAsync.ts
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bundler seemed like it wasn't tree-shaking properly when this file was combined with src/feedback.ts. Splitting them apart resulted in the bundle-sizes I expected

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { buildFeedbackIntegration } from '@sentry-internal/feedback';
import { lazyLoadIntegration } from './utils/lazyLoadIntegration';

// This is for users who want to have a lazy-loaded feedback widget
export const feedbackAsyncIntegration = buildFeedbackIntegration({
lazyLoadIntegration,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole buildFeedbackIntegration exists, and is called from inside the browser package, so that we can inject lazyLoadIntegration instead of creating a circular dependency between @sentry/browser and @sentry/feedback.

});
8 changes: 2 additions & 6 deletions packages/browser/src/index.bundle.feedback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@ import { browserTracingIntegrationShim, replayIntegrationShim } from '@sentry-in

export * from './index.bundle.base';

export {
feedbackIntegration,
feedbackModalIntegration,
feedbackScreenshotIntegration,
getFeedback,
} from '@sentry-internal/feedback';
export { feedbackIntegration } from './feedback';
Copy link
Member Author

@ryan953 ryan953 Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that we're using feedbackIntegration as the default here, not feedbackAsyncIntegration. (same in the othe bundle configs)
So CDN users, by default, won't have async loading.

This is something we can talk about and change outside this PR

export { getFeedback } from '@sentry-internal/feedback';

export { browserTracingIntegrationShim as browserTracingIntegration, replayIntegrationShim as replayIntegration };
8 changes: 2 additions & 6 deletions packages/browser/src/index.bundle.tracing.replay.feedback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,8 @@ export {
setMeasurement,
} from '@sentry/core';

export {
feedbackIntegration,
feedbackModalIntegration,
feedbackScreenshotIntegration,
getFeedback,
} from '@sentry-internal/feedback';
export { feedbackIntegration } from './feedback';
export { getFeedback } from '@sentry-internal/feedback';

export {
browserTracingIntegration,
Expand Down
5 changes: 2 additions & 3 deletions packages/browser/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,9 @@ export type {

export { replayCanvasIntegration } from '@sentry-internal/replay-canvas';

export { feedbackIntegration } from './feedback';
export { feedbackAsyncIntegration } from './feedbackAsync';
export {
feedbackIntegration,
feedbackModalIntegration,
feedbackScreenshotIntegration,
getFeedback,
sendFeedback,
} from '@sentry-internal/feedback';
Expand Down
2 changes: 1 addition & 1 deletion packages/feedback/rollup.bundle.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ export default [
...makeBundleConfigVariants(
makeBaseBundleConfig({
bundleType: 'addon',
entrypoints: ['src/index.bundle.ts'],
entrypoints: ['src/index.ts'],
jsVersion: 'es6',
licenseTitle: '@sentry-internal/feedback',
outputFileBase: () => 'bundles/feedback',
Expand Down
6 changes: 4 additions & 2 deletions packages/feedback/src/core/getFeedback.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { getCurrentScope } from '@sentry/core';
import { getFeedback } from './getFeedback';
import { feedbackIntegration } from './integration';
import { buildFeedbackIntegration } from './integration';
import { mockSdk } from './mockSdk';

describe('getFeedback', () => {
Expand All @@ -25,7 +25,9 @@ describe('getFeedback', () => {
});

it('works with a client with Feedback', () => {
const feedback = feedbackIntegration();
const feedback = buildFeedbackIntegration({
lazyLoadIntegration: jest.fn(),
});

mockSdk({
sentryOptions: {
Expand Down
8 changes: 5 additions & 3 deletions packages/feedback/src/core/getFeedback.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { getClient } from '@sentry/core';
import type { feedbackIntegration } from './integration';
import type { buildFeedbackIntegration } from './integration';

type FeedbackIntegration = ReturnType<typeof buildFeedbackIntegration>;

/**
* This is a small utility to get a type-safe instance of the Feedback integration.
*/
export function getFeedback(): ReturnType<typeof feedbackIntegration> | undefined {
export function getFeedback(): ReturnType<FeedbackIntegration> | undefined {
const client = getClient();
return client && client.getIntegrationByName<ReturnType<typeof feedbackIntegration>>('Feedback');
return client && client.getIntegrationByName('Feedback');
}
Loading
Loading