diff --git a/.size-limit.js b/.size-limit.js index 0b03cc41e003..83a0919cf015 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -225,6 +225,19 @@ module.exports = [ limit: '35 KB', disablePlugins: ['@size-limit/esbuild'], }, + // Remix 3 browser SDK (ESM) + { + // Every export, not a named import: Remix 3 has no bundler, so an app ships every byte of this + // file. The budget is what keeps `src/v3/index.client.ts` a named list. One added + // `export * from '@sentry/browser'` measures 134 KB here, and more on the wire, because a real app + // has no bundler to shake it. + name: '@sentry/remix (Remix 3 client bundle)', + path: 'packages/remix/build/esm/v3/client-bundle.js', + import: '*', + gzip: true, + limit: '56 KB', + disablePlugins: ['@size-limit/esbuild'], + }, // Browser CDN bundles { name: 'CDN Bundle', diff --git a/dev-packages/e2e-tests/test-applications/remix-v3/app/actions/controller.tsx b/dev-packages/e2e-tests/test-applications/remix-v3/app/actions/controller.tsx index 756115577e68..4999bd4127cc 100644 --- a/dev-packages/e2e-tests/test-applications/remix-v3/app/actions/controller.tsx +++ b/dev-packages/e2e-tests/test-applications/remix-v3/app/actions/controller.tsx @@ -17,6 +17,24 @@ function HomePage(handle: Handle>) {

Sentry Remix 3

+ {/* No `data-rmx-document`, so the runtime intercepts this through the Navigation API. */} + + User + + + + ); +} + +function UserPage(handle: Handle<{ id?: string }>) { + return () => ( + + + + User + + +

User {handle.props.id}

); @@ -31,7 +49,7 @@ export default createController(routes, { return context.render(); }, user(context) { - return Response.json({ id: context.params.id }); + return context.render(); }, teapot() { return new Response("I'm a teapot", { status: 418 }); diff --git a/dev-packages/e2e-tests/test-applications/remix-v3/app/actions/public/entry.ts b/dev-packages/e2e-tests/test-applications/remix-v3/app/actions/public/entry.ts index 5f0ebdc1248e..d4cc91b051d5 100644 --- a/dev-packages/e2e-tests/test-applications/remix-v3/app/actions/public/entry.ts +++ b/dev-packages/e2e-tests/test-applications/remix-v3/app/actions/public/entry.ts @@ -1,9 +1,19 @@ +import * as Sentry from '@sentry/remix/v3/client'; import { run } from 'remix/ui'; -// No Sentry here yet: `@sentry/remix/v3/client` does not export `init` until the browser SDK lands. +// Not Node. The asset server substitutes this when it compiles the module, from the `define` map in +// `app/assets.ts`. +declare const process: { env: Record }; + +Sentry.init({ + dsn: process.env.E2E_TEST_DSN, + tunnel: 'http://localhost:3061/', + tracesSampleRate: 1.0, +}); + export const app = run({ async loadModule(moduleUrl, exportName) { - let mod = await import(moduleUrl); + const mod = await import(moduleUrl); return mod[exportName]; }, }); diff --git a/dev-packages/e2e-tests/test-applications/remix-v3/app/assets.ts b/dev-packages/e2e-tests/test-applications/remix-v3/app/assets.ts index a3c153578aa6..7c89e32313c7 100644 --- a/dev-packages/e2e-tests/test-applications/remix-v3/app/assets.ts +++ b/dev-packages/e2e-tests/test-applications/remix-v3/app/assets.ts @@ -7,6 +7,13 @@ export const assets = createAssetServer({ allowPackages: ['remix', '@sentry/remix'], minify: true, watch: false, + scripts: { + // No bundler means no build time env inlining, so `define` is the only way to get configuration + // into a browser module. The asset server substitutes these when it compiles. + define: { + 'process.env.E2E_TEST_DSN': JSON.stringify(process.env.E2E_TEST_DSN), + }, + }, }); const entry = 'app/actions/public/entry.ts'; diff --git a/dev-packages/e2e-tests/test-applications/remix-v3/tests/client.test.ts b/dev-packages/e2e-tests/test-applications/remix-v3/tests/client.test.ts new file mode 100644 index 000000000000..60cc59e24379 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/remix-v3/tests/client.test.ts @@ -0,0 +1,35 @@ +import { expect, test } from '@playwright/test'; +import { getSpanOp, waitForStreamedSpan } from '@sentry-internal/test-utils'; + +const APP_NAME = 'remix-v3'; + +test('sends a pageload span', async ({ page }) => { + const spanPromise = waitForStreamedSpan(APP_NAME, span => getSpanOp(span) === 'pageload' && span.is_segment === true); + + await page.goto('/'); + + const span = await spanPromise; + // Ordinary document load, so this span comes from the upstream integration and takes the low + // cardinality streaming name. + expect(span.name).toBe('Pageload'); + expect(span.attributes?.['sentry.origin']?.value).toBe('auto.pageload.browser'); +}); + +test('sends a navigation span for a link the runtime intercepts', async ({ page }) => { + await page.goto('/'); + + // Selected by origin, not just op. `remix/ui` never touches History, so a span from the upstream + // handler would mean this SDK did not produce it. + const spanPromise = waitForStreamedSpan( + APP_NAME, + span => + getSpanOp(span) === 'navigation' && span.attributes?.['sentry.origin']?.value === 'auto.navigation.remix_v3', + ); + + await page.locator('#to-user').click(); + await expect(page.locator('#user')).toBeVisible(); + + const span = await spanPromise; + expect(span.is_segment).toBe(true); + expect(span.name).toBe('Navigation'); +}); diff --git a/packages/remix/package.json b/packages/remix/package.json index b8079744d232..d2f8d37f4933 100644 --- a/packages/remix/package.json +++ b/packages/remix/package.json @@ -61,7 +61,7 @@ "./v3/client": { "import": { "types": "./build/types/v3/index.client.d.ts", - "default": "./build/esm/v3/index.client.js" + "default": "./build/esm/v3/client-bundle.js" } }, "./v3/node": { diff --git a/packages/remix/rollup.npm.config.mjs b/packages/remix/rollup.npm.config.mjs index d6992abc3302..e4938499f1ec 100644 --- a/packages/remix/rollup.npm.config.mjs +++ b/packages/remix/rollup.npm.config.mjs @@ -1,3 +1,5 @@ +/* eslint-disable import/no-named-as-default */ +import nodeResolve from '@rollup/plugin-node-resolve'; import { defineConfig } from 'rollup'; import { makeBaseNPMConfig, makeNPMConfigVariants, makeOrchestrionLoader } from '@sentry-internal/rollup-utils'; @@ -8,6 +10,39 @@ const v3NodeEntry = defineConfig({ output: { format: 'esm', file: 'build/v3-node.mjs' }, }); +/** + * The Remix 3 browser entry, bundled into a single self contained ES module. + * + * Remix 3 has no bundler. Its asset server serves one HTTP request per module and never drops dead + * code, because it only ever looks at one module at a time. The ordinary `preserveModules` output + * therefore costs an app 255 requests and about 364 KB gzipped of `@sentry/*`; bundled here it is 2 + * requests and 54 KB. Publish time is the only place that reduction can happen. + * + * Runs over `build/esm/v3/index.client.js` rather than the TypeScript source, to reuse the transpilation + * the main config already did. That is why it has to come last in this array. + * + * Left unminified and without a source map on purpose. The asset server minifies what it serves and + * builds its own map chain, so a pre-minified file with its own map would add a second chain for + * nothing. + */ +const v3ClientBundle = defineConfig({ + input: 'build/esm/v3/index.client.js', + external: id => id === 'remix' || id.startsWith('@remix-run/'), + treeshake: { moduleSideEffects: false, propertyReadSideEffects: false }, + plugins: [nodeResolve({ browser: true, exportConditions: ['browser', 'import', 'default'] })], + // Emitted inside `build/esm/v3/`, not at `build/`: the one import it keeps is the relative path to + // the channel shim, which only resolves from there. + output: { file: 'build/esm/v3/client-bundle.js', format: 'esm' }, + onwarn(warning, warn) { + // `this` is undefined in the bundled output of some dependencies, and the graph has cycles. Both + // are harmless here and would otherwise bury real warnings. + if (warning.code === 'THIS_IS_UNDEFINED' || warning.code === 'CIRCULAR_DEPENDENCY') { + return; + } + warn(warning); + }, +}); + // We rely on esbuild's defaults for JSX (`jsx: 'transform'` = classic runtime, no // __self/__source attributes). React 19 prefers the new automatic transform, but switching // to it would break React 17 support — so we intentionally stay on classic for now. @@ -39,4 +74,5 @@ export default [ }), ), ...makeOrchestrionLoader('./build'), + v3ClientBundle, ]; diff --git a/packages/remix/src/v3/client/browserTracingIntegration.ts b/packages/remix/src/v3/client/browserTracingIntegration.ts new file mode 100644 index 000000000000..c9832db5c4cd --- /dev/null +++ b/packages/remix/src/v3/client/browserTracingIntegration.ts @@ -0,0 +1,106 @@ +import { + browserTracingIntegration as originalBrowserTracingIntegration, + startBrowserTracingNavigationSpan, + WINDOW, +} from '@sentry/browser'; +import { SENTRY_SEGMENT_NAME_SOURCE } from '@sentry/conventions/attributes'; +import { + type Client, + hasSpanStreamingEnabled, + type Integration, + NAVIGATION_SPAN_NAME_FALLBACK, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, +} from '@sentry/core'; + +type Options = Parameters[0]; + +/** + * Browser tracing for Remix 3. + * + * Page loads stay with the upstream integration, because they are ordinary document loads. Navigations + * do not: `remix/ui` intercepts links and form submissions through the Navigation API and never touches + * History, so the upstream handler never fires. + */ +export function browserTracingIntegration(options: Options = {}): Integration { + const integration = originalBrowserTracingIntegration({ ...options, instrumentNavigation: false }); + + return { + ...integration, + afterAllSetup(client) { + integration.afterAllSetup(client); + + if (options.instrumentNavigation !== false) { + instrumentNavigationApi(client); + } + }, + }; +} + +function instrumentNavigationApi(client: Client): void { + const navigation = (WINDOW as WindowWithNavigation).navigation; + if (!navigation) { + // Without the Navigation API the runtime does full document loads, which are page loads already. + return; + } + + navigation.addEventListener('navigate', event => { + const url = event.destination?.url; + if (!url || !isRuntimeNavigation(event, url)) { + return; + } + + startBrowserTracingNavigationSpan( + client, + { + // Remix 3 gives the browser no route to name this after: `remix/ui` exposes no matched route + // and never matches client side. Passing the server's pattern down is tracked in (#24872). + name: hasSpanStreamingEnabled(client) ? NAVIGATION_SPAN_NAME_FALLBACK : pathnameOf(url) || '/', + attributes: { + [SENTRY_SEGMENT_NAME_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.remix_v3', + }, + }, + // The span needs the destination: `location` still points at the previous page until the + // navigation finishes. + { url }, + ); + }); +} + +/** + * Whether the runtime will keep this navigation inside the current document. + * + * The same three conditions `startNavigationListener` in `@remix-run/ui` checks before intercepting. + * What it declines becomes a new document load, which already gets a page load span, so a navigation + * span here would count the same click twice. + */ +function isRuntimeNavigation(event: NavigateEventLike, url: string): boolean { + // `'remix-document-reload'` is the `info` value the runtime tags its own document reloads with. + return event.canIntercept && event.info !== 'remix-document-reload' && isSameOrigin(url); +} + +function isSameOrigin(url: string): boolean { + try { + return new URL(url).origin === WINDOW.location?.origin; + } catch { + return false; + } +} + +function pathnameOf(url: string): string | undefined { + try { + return new URL(url).pathname; + } catch { + return undefined; + } +} + +interface NavigateEventLike extends Event { + canIntercept: boolean; + info?: unknown; + destination?: { url: string }; +} + +type WindowWithNavigation = typeof WINDOW & { + navigation?: { addEventListener(type: 'navigate', listener: (event: NavigateEventLike) => void): void }; +}; diff --git a/packages/remix/src/v3/client/sdk.ts b/packages/remix/src/v3/client/sdk.ts new file mode 100644 index 000000000000..463de70e504b --- /dev/null +++ b/packages/remix/src/v3/client/sdk.ts @@ -0,0 +1,28 @@ +import type { BrowserOptions } from '@sentry/browser'; +import { getDefaultIntegrations as getBrowserDefaultIntegrations, init as browserInit } from '@sentry/browser'; +import { applySdkMetadata, type Client, type Integration } from '@sentry/core'; + +import { browserTracingIntegration } from './browserTracingIntegration'; + +/** + * Default integrations for the Remix 3 client SDK. + * + * Browser tracing is added here rather than left to the app, because the Navigation API variant is the + * only one that reports anything in Remix 3. Everything else is plain `@sentry/browser`. Nothing from + * `@sentry/react` applies: `remix/ui` is its own runtime, with no React and no reconciler to hook. + */ +export function getDefaultIntegrations(options: BrowserOptions): Integration[] { + return [...getBrowserDefaultIntegrations(options), browserTracingIntegration()]; +} + +/** Initialize the Sentry Remix 3 SDK in the browser. */ +export function init(options: BrowserOptions): Client | undefined { + const opts = { + ...options, + defaultIntegrations: options.defaultIntegrations ?? getDefaultIntegrations(options), + }; + + applySdkMetadata(opts, 'remix', ['remix', 'browser']); + + return browserInit(opts); +} diff --git a/packages/remix/src/v3/index.client.ts b/packages/remix/src/v3/index.client.ts index 7cdc7036be5a..761d76bd0e54 100644 --- a/packages/remix/src/v3/index.client.ts +++ b/packages/remix/src/v3/index.client.ts @@ -1,8 +1,76 @@ -// A named list rather than `export * from '@sentry/browser'`: Remix 3 has no bundler, so a wildcard -// makes every export reachable and nothing can be tree shaken out of the served module graph. +// A named list rather than `export * from '@sentry/browser'`. // -// `init` is withheld until the Remix 3 browser SDK lands. Re-exporting `@sentry/browser`'s would give -// an app History based tracing, which yields no navigation spans in Remix 3, so it would look -// configured while reporting nothing. -export { captureException, captureMessage } from '@sentry/browser'; +// Remix 3 has no bundler, so this list is what gets tree shaken. A wildcard re-export would reach every +// export of `@sentry/browser`, including Replay, Feedback and the full attribute tables, and none of it +// could be dropped. Add whatever is needed, but keep it a named list. The size limit budget for +// `client-bundle.js` is what enforces that. +// +// Everything comes from `@sentry/browser`, never `@sentry/core`, even where `@sentry/browser` only +// passes a symbol through. The two packages export different `startSpan`, `startInactiveSpan` and +// `startSpanManual`, and only `@sentry/browser`'s installs span streaming on first use. +export { + addBreadcrumb, + addEventProcessor, + addIntegration, + breadcrumbsIntegration, + browserApiErrorsIntegration, + BrowserClient, + captureEvent, + captureException, + captureFeedback, + captureMessage, + captureSession, + close, + continueTrace, + dedupeIntegration, + defaultStackParser, + endSession, + eventFiltersIntegration, + flush, + functionToStringIntegration, + getActiveSpan, + getClient, + getCurrentScope, + getGlobalScope, + getIsolationScope, + getRootSpan, + getTraceData, + globalHandlersIntegration, + httpContextIntegration, + isEnabled, + isInitialized, + lastEventId, + linkedErrorsIntegration, + logger, + makeFetchTransport, + metrics, + parameterize, + Scope, + SDK_VERSION, + setAttribute, + setAttributes, + setContext, + setCurrentClient, + setExtra, + setExtras, + setTag, + setTags, + setUser, + spanToJSON, + spanToTraceHeader, + startInactiveSpan, + startSession, + startSpan, + startSpanManual, + suppressTracing, + updateSpanName, + WINDOW, + withActiveSpan, + withIsolationScope, + withScope, +} from '@sentry/browser'; + export type { BrowserOptions } from '@sentry/browser'; + +export { getDefaultIntegrations, init } from './client/sdk'; +export { browserTracingIntegration } from './client/browserTracingIntegration'; diff --git a/packages/remix/test/v3/browserTracingIntegration.test.ts b/packages/remix/test/v3/browserTracingIntegration.test.ts new file mode 100644 index 000000000000..2a78829b1107 --- /dev/null +++ b/packages/remix/test/v3/browserTracingIntegration.test.ts @@ -0,0 +1,119 @@ +import type { Client } from '@sentry/core'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const startBrowserTracingNavigationSpan = vi.fn(); +const upstreamIntegration = { name: 'BrowserTracing', afterAllSetup: vi.fn() }; +const upstreamOptions: { instrumentNavigation?: boolean }[] = []; + +vi.mock('@sentry/browser', () => ({ + browserTracingIntegration: (options: { instrumentNavigation?: boolean }) => { + upstreamOptions.push(options); + return upstreamIntegration; + }, + startBrowserTracingNavigationSpan: (...args: unknown[]) => startBrowserTracingNavigationSpan(...args), + WINDOW: globalThis, +})); + +const { browserTracingIntegration } = await import('../../src/v3/client/browserTracingIntegration'); + +interface FakeNavigateEvent { + canIntercept: boolean; + info?: unknown; + destination?: { url: string }; +} + +const client = { getOptions: () => ({ traceLifecycle: 'stream' }) } as unknown as Client; + +/** Sets up the integration over a fake Navigation API and returns a way to fire `navigate` events. */ +function setup(options = {}): (event: FakeNavigateEvent) => void { + let listener: ((event: FakeNavigateEvent) => void) | undefined; + + Object.assign(globalThis, { + location: { origin: 'https://app.test', pathname: '/' }, + navigation: { + addEventListener: (_type: string, fn: (event: FakeNavigateEvent) => void) => { + listener = fn; + }, + }, + }); + + browserTracingIntegration(options).afterAllSetup?.(client); + + return event => listener?.(event); +} + +describe('browserTracingIntegration', () => { + beforeEach(() => { + startBrowserTracingNavigationSpan.mockClear(); + upstreamOptions.length = 0; + }); + + it('leaves page loads to the upstream integration and takes over navigation', () => { + setup(); + + expect(upstreamOptions[0]).toEqual({ instrumentNavigation: false }); + }); + + it('starts a navigation span for an intercepted navigation', () => { + const navigate = setup(); + + navigate({ canIntercept: true, destination: { url: 'https://app.test/users/12345' } }); + + expect(startBrowserTracingNavigationSpan).toHaveBeenCalledWith( + client, + { + // Low cardinality, because the browser has no route patterns. + name: 'Navigation', + attributes: { + 'sentry.segment.name.source': 'url', + 'sentry.origin': 'auto.navigation.remix_v3', + }, + }, + { url: 'https://app.test/users/12345' }, + ); + }); + + // Each of these becomes a new document load, which the upstream integration reports as a page load, + // so a navigation span would count the same action twice. + it.each([ + ['the runtime cannot intercept it', { canIntercept: false, destination: { url: 'https://app.test/x' } }], + [ + "it is the runtime's own document reload", + { canIntercept: true, info: 'remix-document-reload', destination: { url: 'https://app.test/x' } }, + ], + ['it leaves the origin', { canIntercept: true, destination: { url: 'https://elsewhere.test/x' } }], + ['there is no destination', { canIntercept: true }], + ])('starts no navigation span when %s', (_why, event) => { + const navigate = setup(); + + navigate(event as FakeNavigateEvent); + + expect(startBrowserTracingNavigationSpan).not.toHaveBeenCalled(); + }); + + it('names the span after the path when span streaming is off', () => { + const staticClient = { getOptions: () => ({ traceLifecycle: 'static' }) } as unknown as Client; + let listener: ((event: FakeNavigateEvent) => void) | undefined; + Object.assign(globalThis, { + location: { origin: 'https://app.test', pathname: '/' }, + navigation: { addEventListener: (_t: string, fn: typeof listener) => void (listener = fn) }, + }); + browserTracingIntegration().afterAllSetup?.(staticClient); + + listener?.({ canIntercept: true, destination: { url: 'https://app.test/users/12345?q=1' } }); + + expect(startBrowserTracingNavigationSpan).toHaveBeenCalledWith( + staticClient, + expect.objectContaining({ name: '/users/12345' }), + { url: 'https://app.test/users/12345?q=1' }, + ); + }); + + it('starts no navigation span when instrumentNavigation is off', () => { + const navigate = setup({ instrumentNavigation: false }); + + navigate({ canIntercept: true, destination: { url: 'https://app.test/users/1' } }); + + expect(startBrowserTracingNavigationSpan).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/remix/test/v3/sdk.test.ts b/packages/remix/test/v3/sdk.test.ts new file mode 100644 index 000000000000..1436966ddb91 --- /dev/null +++ b/packages/remix/test/v3/sdk.test.ts @@ -0,0 +1,71 @@ +import type * as SentryBrowser from '@sentry/browser'; +import type { BrowserOptions } from '@sentry/browser'; +import { describe, expect, it, vi } from 'vitest'; + +const browserInit = vi.fn(); + +// Only `init` is replaced. Everything else stays real, so the integration assertion below is checked +// against the actual `@sentry/browser` defaults. +vi.mock('@sentry/browser', async importOriginal => ({ + ...(await importOriginal()), + init: (options: BrowserOptions) => browserInit(options), +})); + +const { getDefaultIntegrations, init } = await import('../../src/v3/client/sdk'); + +describe('getDefaultIntegrations', () => { + // `@sentry/browser`'s own defaults do not include browser tracing today, so this list adds it. If + // that ever changes upstream, a Remix 3 app would quietly end up with two browser tracing + // integrations, one of which reports no navigations at all. + it('adds exactly one browser tracing integration', () => { + const integrations = getDefaultIntegrations({}); + + expect(integrations.filter(integration => integration.name === 'BrowserTracing')).toHaveLength(1); + }); +}); + +describe('init', () => { + // The Remix 3 code ships inside `@sentry/remix`, so that is the package events have to name. Any + // other name is not installable from npm. + it('reports installable package names', () => { + init({}); + + expect(browserInit).toHaveBeenCalledWith( + expect.objectContaining({ + _metadata: { + sdk: { + name: 'sentry.javascript.remix', + version: expect.any(String), + packages: [ + { name: 'npm:@sentry/remix', version: expect.any(String) }, + { name: 'npm:@sentry/browser', version: expect.any(String) }, + ], + }, + }, + }), + ); + }); +}); + +describe('the client entry', () => { + // For the span start APIs the two packages export different implementations, and only + // `@sentry/browser`'s installs span streaming on first use. Taking one from `@sentry/core` is + // invisible until an app replaces the default integrations, because browser tracing installs span + // streaming anyway. + it('re-exports @sentry/browser, never a @sentry/core lookalike', async () => { + const [entry, browser] = await Promise.all([ + import('../../src/v3/index.client'), + vi.importActual('@sentry/browser'), + ]); + + // The three the Remix 3 SDK deliberately replaces. + const overridden = ['init', 'getDefaultIntegrations', 'browserTracingIntegration']; + const shared = Object.keys(entry).filter(name => name in browser && !overridden.includes(name)); + const wrongSource = shared.filter( + name => entry[name as keyof typeof entry] !== browser[name as keyof typeof browser], + ); + + expect(shared.length).toBeGreaterThan(20); + expect(wrongSource).toEqual([]); + }); +});