-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(nextjs): Add experimental support for react component annotation in Turbopack #19604
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| 'use client'; | ||
|
|
||
| import * as Sentry from '@sentry/nextjs'; | ||
|
|
||
| export default function ComponentAnnotationTestPage() { | ||
| return ( | ||
| <div> | ||
| <button | ||
| id="annotated-btn" | ||
| onClick={() => { | ||
| Sentry.captureException(new Error('component-annotation-test')); | ||
| }} | ||
| > | ||
| Click Me | ||
| </button> | ||
| </div> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import { expect, test } from '@playwright/test'; | ||
| import { waitForError } from '@sentry-internal/test-utils'; | ||
|
|
||
| const isWebpackDev = process.env.TEST_ENV === 'development-webpack'; | ||
|
|
||
| test('React component annotation adds data-sentry-component attributes (Turbopack)', async ({ page }) => { | ||
| test.skip(isWebpackDev, 'Only relevant for Turbopack builds'); | ||
|
|
||
| await page.goto('/component-annotation'); | ||
|
|
||
| const button = page.locator('#annotated-btn'); | ||
| await expect(button).toBeVisible(); | ||
|
|
||
| // Set up error listener before clicking | ||
| const errorPromise = waitForError('nextjs-16', errorEvent => { | ||
| return errorEvent?.exception?.values?.some(value => value.value === 'component-annotation-test') ?? false; | ||
| }); | ||
|
|
||
| await button.click(); | ||
| const errorEvent = await errorPromise; | ||
|
|
||
| expect(errorEvent.exception?.values?.[0]?.value).toBe('component-annotation-test'); | ||
|
|
||
| // In production, TEST_ENV=production is shared by both turbopack and webpack variants. | ||
| // The component annotation loader only runs in Turbopack builds, so use the independent | ||
| // turbopack tag (set by the SDK based on build metadata) to gate assertions rather than | ||
| // checking the feature's own output, which would silently pass on regression. | ||
| if (errorEvent.tags?.turbopack) { | ||
| const annotatedEl = page.locator('[data-sentry-component="ComponentAnnotationTestPage"]'); | ||
| await expect(annotatedEl).toBeVisible(); | ||
|
|
||
| const clickBreadcrumb = errorEvent.breadcrumbs?.find(bc => bc.category === 'ui.click'); | ||
| expect(clickBreadcrumb?.data?.['ui.component_name']).toBe('ComponentAnnotationTestPage'); | ||
| } | ||
chargome marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import { createComponentNameAnnotateHooks } from '@sentry/bundler-plugin-core'; | ||
| import type { LoaderThis } from './types'; | ||
|
|
||
| export type ComponentAnnotationLoaderOptions = { | ||
| ignoredComponents?: string[]; | ||
| }; | ||
|
|
||
| /** | ||
| * Turbopack loader that annotates React components with `data-sentry-component`, | ||
| * `data-sentry-element`, and `data-sentry-source-file` attributes. | ||
| * | ||
| * This is the Turbopack equivalent of what `@sentry/webpack-plugin` does | ||
| * via the `reactComponentAnnotation` option and `@sentry/babel-plugin-component-annotate`. | ||
| * | ||
| * Options: | ||
| * - `ignoredComponents`: List of component names to exclude from annotation. | ||
| */ | ||
| export default function componentAnnotationLoader( | ||
| this: LoaderThis<ComponentAnnotationLoaderOptions>, | ||
| userCode: string, | ||
| ): void { | ||
| const options = 'getOptions' in this ? this.getOptions() : this.query; | ||
| const ignoredComponents = options.ignoredComponents ?? []; | ||
|
|
||
| // We do not want to cache results across builds | ||
| this.cacheable(false); | ||
|
|
||
| const callback = this.async() ?? this.callback; | ||
|
|
||
| const hooks = createComponentNameAnnotateHooks(ignoredComponents, false); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Q: I guess with loaders we can only use the component-based plugin and have to set
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we could actually use both but went with the lower html impact – safer default imo
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah fine for now I think |
||
|
|
||
| hooks | ||
| .transform(userCode, this.resourcePath) | ||
| .then(result => { | ||
| if (result) { | ||
| callback(null, result.code, result.map); | ||
| } else { | ||
| callback(null, userCode); | ||
| } | ||
| }) | ||
| .catch(() => { | ||
| // On error, pass through the original code gracefully | ||
| callback(null, userCode); | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import type { ComponentAnnotationLoaderOptions } from '../../../src/config/loaders/componentAnnotationLoader'; | ||
| import componentAnnotationLoader from '../../../src/config/loaders/componentAnnotationLoader'; | ||
| import type { LoaderThis } from '../../../src/config/loaders/types'; | ||
|
|
||
| const { mockTransform, mockCreateHooks } = vi.hoisted(() => { | ||
| const mockTransform = vi.fn(); | ||
| const mockCreateHooks = vi.fn().mockReturnValue({ transform: mockTransform }); | ||
| return { mockTransform, mockCreateHooks }; | ||
| }); | ||
|
|
||
| vi.mock('@sentry/bundler-plugin-core', () => ({ | ||
| createComponentNameAnnotateHooks: mockCreateHooks, | ||
| })); | ||
|
|
||
| function createMockLoaderContext( | ||
| options: ComponentAnnotationLoaderOptions = {}, | ||
| resourcePath = '/app/components/Button.tsx', | ||
| ): LoaderThis<ComponentAnnotationLoaderOptions> & { callback: ReturnType<typeof vi.fn> } { | ||
| const callback = vi.fn(); | ||
| return { | ||
| resourcePath, | ||
| addDependency: vi.fn(), | ||
| cacheable: vi.fn(), | ||
| async: vi.fn().mockReturnValue(callback), | ||
| callback, | ||
| getOptions: vi.fn().mockReturnValue(options), | ||
| }; | ||
| } | ||
|
|
||
| describe('componentAnnotationLoader', () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| mockTransform.mockReset(); | ||
| mockCreateHooks.mockReturnValue({ transform: mockTransform }); | ||
| }); | ||
|
|
||
| it('calls this.async() and uses callback with transformed code and source map', async () => { | ||
| const mockResult = { | ||
| code: 'transformed code', | ||
| map: { version: 3, sources: ['Button.tsx'] }, | ||
| }; | ||
| mockTransform.mockResolvedValue(mockResult); | ||
|
|
||
| const ctx = createMockLoaderContext(); | ||
| componentAnnotationLoader.call(ctx, 'original code'); | ||
|
|
||
| await new Promise(resolve => setTimeout(resolve, 0)); | ||
|
|
||
| expect(ctx.async).toHaveBeenCalled(); | ||
| expect(ctx.callback).toHaveBeenCalledWith(null, 'transformed code', { version: 3, sources: ['Button.tsx'] }); | ||
| }); | ||
|
|
||
| it('passes through original code when transform returns null', async () => { | ||
| mockTransform.mockResolvedValue(null); | ||
|
|
||
| const ctx = createMockLoaderContext(); | ||
| componentAnnotationLoader.call(ctx, 'original code'); | ||
|
|
||
| await new Promise(resolve => setTimeout(resolve, 0)); | ||
|
|
||
| expect(ctx.callback).toHaveBeenCalledWith(null, 'original code'); | ||
| }); | ||
|
|
||
| it('passes through original code on transform error', async () => { | ||
| mockTransform.mockRejectedValue(new Error('babel error')); | ||
|
|
||
| const ctx = createMockLoaderContext(); | ||
| componentAnnotationLoader.call(ctx, 'original code'); | ||
|
|
||
| await new Promise(resolve => setTimeout(resolve, 0)); | ||
|
|
||
| expect(ctx.callback).toHaveBeenCalledWith(null, 'original code'); | ||
| }); | ||
|
|
||
| it('sets cacheable(false)', () => { | ||
| mockTransform.mockResolvedValue(null); | ||
|
|
||
| const ctx = createMockLoaderContext(); | ||
| componentAnnotationLoader.call(ctx, 'original code'); | ||
|
|
||
| expect(ctx.cacheable).toHaveBeenCalledWith(false); | ||
| }); | ||
|
|
||
| it('reads options via getOptions() (webpack 5)', async () => { | ||
| mockTransform.mockResolvedValue(null); | ||
|
|
||
| const ctx = createMockLoaderContext({ ignoredComponents: ['Header'] }); | ||
| componentAnnotationLoader.call(ctx, 'original code'); | ||
|
|
||
| await new Promise(resolve => setTimeout(resolve, 0)); | ||
|
|
||
| expect(mockCreateHooks).toHaveBeenCalledWith(['Header'], false); | ||
| }); | ||
|
|
||
| it('reads options via this.query (webpack 4)', async () => { | ||
| mockTransform.mockResolvedValue(null); | ||
|
|
||
| const callback = vi.fn(); | ||
| const ctx = { | ||
| resourcePath: '/app/components/Button.tsx', | ||
| addDependency: vi.fn(), | ||
| cacheable: vi.fn(), | ||
| async: vi.fn().mockReturnValue(callback), | ||
| callback, | ||
| query: { ignoredComponents: ['Footer'] }, | ||
| } as unknown as LoaderThis<ComponentAnnotationLoaderOptions>; | ||
|
|
||
| componentAnnotationLoader.call(ctx, 'original code'); | ||
|
|
||
| await new Promise(resolve => setTimeout(resolve, 0)); | ||
|
|
||
| expect(mockCreateHooks).toHaveBeenCalledWith(['Footer'], false); | ||
| }); | ||
|
|
||
| it('defaults ignoredComponents to empty array', async () => { | ||
| mockTransform.mockResolvedValue(null); | ||
|
|
||
| const ctx = createMockLoaderContext({}); | ||
| componentAnnotationLoader.call(ctx, 'original code'); | ||
|
|
||
| await new Promise(resolve => setTimeout(resolve, 0)); | ||
|
|
||
| expect(mockCreateHooks).toHaveBeenCalledWith([], false); | ||
| }); | ||
|
|
||
| it('passes resourcePath to transform', async () => { | ||
| mockTransform.mockResolvedValue(null); | ||
|
|
||
| const ctx = createMockLoaderContext({}, '/app/pages/Home.tsx'); | ||
| componentAnnotationLoader.call(ctx, 'some code'); | ||
|
|
||
| await new Promise(resolve => setTimeout(resolve, 0)); | ||
|
|
||
| expect(mockTransform).toHaveBeenCalledWith('some code', '/app/pages/Home.tsx'); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.