getsentry · lforst · Oct 28, 2024 · Oct 28, 2024 · Oct 28, 2024 · Oct 28, 2024
diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-13/sentry.client.config.ts
@@ -1,3 +1,5 @@
+'use client';
+
 import * as Sentry from '@sentry/nextjs';
 
 Sentry.init({

diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/sentry.client.config.ts
@@ -1,3 +1,5 @@
+'use client';
+
 import * as Sentry from '@sentry/nextjs';
 
 Sentry.init({

diff --git a/dev-packages/e2e-tests/test-applications/nextjs-15/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-15/sentry.client.config.ts
@@ -1,3 +1,5 @@
+'use client';
+
 import * as Sentry from '@sentry/nextjs';
 
 Sentry.init({

diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/sentry.client.config.ts
@@ -1,3 +1,5 @@
+'use client';
+
 import * as Sentry from '@sentry/nextjs';
 
 Sentry.init({

diff --git a/dev-packages/e2e-tests/test-applications/nextjs-t3/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-t3/sentry.client.config.ts
@@ -1,3 +1,5 @@
+'use client';
+
 import * as Sentry from '@sentry/nextjs';
 
 Sentry.init({

diff --git a/dev-packages/e2e-tests/test-applications/nextjs-turbo/app/layout.tsx b/dev-packages/e2e-tests/test-applications/nextjs-turbo/app/layout.tsx
@@ -1,7 +1,12 @@
+import { HackComponentToRunSideEffectsInSentryClientConfig } from '../sentry.client.config';
+
 export default function Layout({ children }: { children: React.ReactNode }) {
   return (
     <html lang="en">
-      <body>{children}</body>
+      <body>
+        <HackComponentToRunSideEffectsInSentryClientConfig />
+        {children}
+      </body>
     </html>
   );
 }
diff --git a/...ages/[param]/client-trace-propagation.tsx → ...pages-router-client-trace-propagation.tsx b/...ages/[param]/client-trace-propagation.tsx → ...pages-router-client-trace-propagation.tsx
diff --git a/dev-packages/e2e-tests/test-applications/nextjs-turbo/pages/_app.tsx b/dev-packages/e2e-tests/test-applications/nextjs-turbo/pages/_app.tsx
@@ -0,0 +1,6 @@
+import type { AppProps } from 'next/app';
+import '../sentry.client.config';
+
+export default function CustomApp({ Component, pageProps }: AppProps) {
+  return <Component {...pageProps} />;
+}
diff --git a/dev-packages/e2e-tests/test-applications/nextjs-turbo/sentry.client.config.ts b/dev-packages/e2e-tests/test-applications/nextjs-turbo/sentry.client.config.ts
@@ -1,9 +1,17 @@
+'use client';
+
 import * as Sentry from '@sentry/nextjs';
 
-Sentry.init({
-  environment: 'qa', // dynamic sampling bias to keep transactions
-  dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN,
-  tunnel: `http://localhost:3031/`, // proxy server
-  tracesSampleRate: 1.0,
-  sendDefaultPii: true,
-});
+if (typeof window !== 'undefined') {
+  Sentry.init({
+    environment: 'qa', // dynamic sampling bias to keep transactions
+    dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN,
+    tunnel: `http://localhost:3031/`, // proxy server
+    tracesSampleRate: 1.0,
+    sendDefaultPii: true,
+  });
+}
+
+export function HackComponentToRunSideEffectsInSentryClientConfig() {
+  return null;
+}
diff --git a/...-tests/test-applications/nextjs-turbo/tests/pages-router/client-trace-propagation.test.ts b/...-tests/test-applications/nextjs-turbo/tests/pages-router/client-trace-propagation.test.ts
@@ -1,25 +1,20 @@
 import { expect, test } from '@playwright/test';
 import { waitForTransaction } from '@sentry-internal/test-utils';
-import { extractTraceparentData } from '@sentry/utils';
 
 test('Should propagate traces from server to client in pages router', async ({ page }) => {
   const serverTransactionPromise = waitForTransaction('nextjs-turbo', async transactionEvent => {
-    return transactionEvent?.transaction === 'GET /[param]/client-trace-propagation';
+    return transactionEvent?.transaction === 'GET /[param]/pages-router-client-trace-propagation';
   });
 
-  await page.goto(`/123/client-trace-propagation`);
-
-  const sentryTraceLocator = await page.locator('meta[name="sentry-trace"]');
-  const sentryTraceValue = await sentryTraceLocator.getAttribute('content');
-  expect(sentryTraceValue).toMatch(/^[a-f0-9]{32}-[a-f0-9]{16}-[0-1]$/);
-
-  const baggageLocator = await page.locator('meta[name="baggage"]');
-  const baggageValue = await baggageLocator.getAttribute('content');
-  expect(baggageValue).toMatch(/sentry-public_key=/);
+  const pageloadTransactionPromise = waitForTransaction('nextjs-turbo', async transactionEvent => {
+    return transactionEvent?.transaction === '/[param]/pages-router-client-trace-propagation';
+  });
 
-  const traceparentData = extractTraceparentData(sentryTraceValue!);
+  await page.goto(`/123/pages-router-client-trace-propagation`);
 
   const serverTransaction = await serverTransactionPromise;
+  const pageloadTransaction = await pageloadTransactionPromise;
 
-  expect(serverTransaction.contexts?.trace?.trace_id).toBe(traceparentData?.traceId);
+  expect(serverTransaction.contexts?.trace?.trace_id).toBeDefined();
+  expect(pageloadTransaction.contexts?.trace?.trace_id).toBe(serverTransaction.contexts?.trace?.trace_id);
 });
diff --git a/packages/nextjs/src/config/loaders/valueInjectionLoader.ts b/packages/nextjs/src/config/loaders/valueInjectionLoader.ts
@@ -1,26 +1,43 @@
+// Rollup doesn't like if we put the directive regex as a literal (?). No idea why.
+/* eslint-disable @sentry-internal/sdk/no-regexp-constructor */
+
 import type { LoaderThis } from './types';
 
-type LoaderOptions = {
+export type ValueInjectionLoaderOptions = {
   values: Record<string, unknown>;
 };
 
+// We need to be careful not to inject anything before any `"use strict";`s or "use client"s or really any other directive.
+// As an additional complication directives may come after any number of comments.
+// This regex is shamelessly stolen from: https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/7f984482c73e4284e8b12a08dfedf23b5a82f0af/packages/bundler-plugin-core/src/index.ts#L535-L539
+const SKIP_COMMENT_AND_DIRECTIVE_REGEX =
+  // Note: CodeQL complains that this regex potentially has n^2 runtime. This likely won't affect realistic files.
+  // biome-ignore lint/nursery/useRegexLiterals: No user input
+  new RegExp('^(?:\\s*|/\\*(?:.|\\r|\\n)*?\\*/|//.*[\\n\\r])*(?:"[^"]*";?|\'[^\']*\';?)?');
+
 /**
  * Set values on the global/window object at the start of a module.
  *
  * Options:
  *   - `values`: An object where the keys correspond to the keys of the global values to set and the values
  *        correspond to the values of the values on the global object. Values must be JSON serializable.
  */
-export default function valueInjectionLoader(this: LoaderThis<LoaderOptions>, userCode: string): string {
+export default function valueInjectionLoader(this: LoaderThis<ValueInjectionLoaderOptions>, userCode: string): string {
   // We know one or the other will be defined, depending on the version of webpack being used
   const { values } = 'getOptions' in this ? this.getOptions() : this.query;
 
   // We do not want to cache injected values across builds
   this.cacheable(false);
 
-  const injectedCode = Object.entries(values)
-    .map(([key, value]) => `globalThis["${key}"] = ${JSON.stringify(value)};`)
-    .join('\n');
+  // Not putting any newlines in the generated code will decrease the likelihood of sourcemaps breaking
+  const injectedCode =
+    // eslint-disable-next-line prefer-template
+    ';' +
+    Object.entries(values)
+      .map(([key, value]) => `globalThis["${key}"] = ${JSON.stringify(value)};`)
+      .join('');
 
-  return `${injectedCode}\n${userCode}`;
+  return userCode.replace(SKIP_COMMENT_AND_DIRECTIVE_REGEX, match => {
+    return match + injectedCode;
+  });
 }
diff --git a/packages/nextjs/test/config/__snapshots__/valueInjectionLoader.test.ts.snap b/packages/nextjs/test/config/__snapshots__/valueInjectionLoader.test.ts.snap
@@ -0,0 +1,83 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`valueInjectionLoader should correctly insert values for basic config 1`] = `
+"
+      ;globalThis[\\"foo\\"] = \\"bar\\";import * as Sentry from '@sentry/nextjs';
+      Sentry.init();
+    "
+`;
+
+exports[`valueInjectionLoader should correctly insert values with a misplaced directive 1`] = `
+"
+      ;globalThis[\\"foo\\"] = \\"bar\\";console.log('This will render the directive useless');
+      \\"use client\\";
+
+
+
+      import * as Sentry from '@sentry/nextjs';
+      Sentry.init();
+    "
+`;
+
+exports[`valueInjectionLoader should correctly insert values with directive 1`] = `
+"
+      \\"use client\\";globalThis[\\"foo\\"] = \\"bar\\";
+      import * as Sentry from '@sentry/nextjs';
+      Sentry.init();
+    "
+`;
+
+exports[`valueInjectionLoader should correctly insert values with directive and block comments 1`] = `
+"
+      /* test */
+      \\"use client\\";;globalThis[\\"foo\\"] = \\"bar\\";
+      import * as Sentry from '@sentry/nextjs';
+      Sentry.init();
+    "
+`;
+
+exports[`valueInjectionLoader should correctly insert values with directive and inline comments 1`] = `
+"
+      // test
+      \\"use client\\";;globalThis[\\"foo\\"] = \\"bar\\";
+      import * as Sentry from '@sentry/nextjs';
+      Sentry.init();
+    "
+`;
+
+exports[`valueInjectionLoader should correctly insert values with directive and multiline block comments 1`] = `
+"
+      /*
+        test
+      */
+      \\"use client\\";;globalThis[\\"foo\\"] = \\"bar\\";
+      import * as Sentry from '@sentry/nextjs';
+      Sentry.init();
+    "
+`;
+
+exports[`valueInjectionLoader should correctly insert values with directive and multiline block comments and a bunch of whitespace 1`] = `
+"
+      /*
+        test
+      */
+
+
+
+
+      \\"use client\\";;globalThis[\\"foo\\"] = \\"bar\\";
+
+
+
+      import * as Sentry from '@sentry/nextjs';
+      Sentry.init();
+    "
+`;
+
+exports[`valueInjectionLoader should correctly insert values with directive and semicolon 1`] = `
+"
+      \\"use client\\";;globalThis[\\"foo\\"] = \\"bar\\";
+      import * as Sentry from '@sentry/nextjs';
+      Sentry.init();
+    "
+`;
diff --git a/packages/nextjs/test/config/valueInjectionLoader.test.ts b/packages/nextjs/test/config/valueInjectionLoader.test.ts
@@ -0,0 +1,146 @@
+import type { LoaderThis } from '../../src/config/loaders/types';
+import type { ValueInjectionLoaderOptions } from '../../src/config/loaders/valueInjectionLoader';
+import valueInjectionLoader from '../../src/config/loaders/valueInjectionLoader';
+
+const defaultLoaderThis = {
+  addDependency: () => undefined,
+  async: () => undefined,
+  cacheable: () => undefined,
+  callback: () => undefined,
+};
+
+const loaderThis = {
+  ...defaultLoaderThis,
+  resourcePath: './client.config.ts',
+  getOptions() {
+    return {
+      values: {
+        foo: 'bar',
+      },
+    };
+  },
+} satisfies LoaderThis<ValueInjectionLoaderOptions>;
+
+describe('valueInjectionLoader', () => {
+  it('should correctly insert values for basic config', () => {
+    const userCode = `
+      import * as Sentry from '@sentry/nextjs';
+      Sentry.init();
+    `;
+
+    const result = valueInjectionLoader.call(loaderThis, userCode);
+
+    expect(result).toMatchSnapshot();
+    expect(result).toMatch(';globalThis["foo"] = "bar";');
+  });
+
+  it('should correctly insert values with directive', () => {
+    const userCode = `
+      "use client"
+      import * as Sentry from '@sentry/nextjs';
+      Sentry.init();
+    `;
+
+    const result = valueInjectionLoader.call(loaderThis, userCode);
+
+    expect(result).toMatchSnapshot();
+    expect(result).toMatch(';globalThis["foo"] = "bar";');
+  });
+
+  it('should correctly insert values with directive and semicolon', () => {
+    const userCode = `
+      "use client";
+      import * as Sentry from '@sentry/nextjs';
+      Sentry.init();
+    `;
+
+    const result = valueInjectionLoader.call(loaderThis, userCode);
+
+    expect(result).toMatchSnapshot();
+    expect(result).toMatch(';globalThis["foo"] = "bar";');
+  });
+
+  it('should correctly insert values with directive and inline comments', () => {
+    const userCode = `
+      // test
+      "use client";
+      import * as Sentry from '@sentry/nextjs';
+      Sentry.init();
+    `;
+
+    const result = valueInjectionLoader.call(loaderThis, userCode);
+
+    expect(result).toMatchSnapshot();
+    expect(result).toMatch(';globalThis["foo"] = "bar";');
+  });
+
+  it('should correctly insert values with directive and block comments', () => {
+    const userCode = `
+      /* test */
+      "use client";
+      import * as Sentry from '@sentry/nextjs';
+      Sentry.init();
+    `;
+
+    const result = valueInjectionLoader.call(loaderThis, userCode);
+
+    expect(result).toMatchSnapshot();
+    expect(result).toMatch(';globalThis["foo"] = "bar";');
+  });
+
+  it('should correctly insert values with directive and multiline block comments', () => {
+    const userCode = `
+      /*
+        test
+      */
+      "use client";
+      import * as Sentry from '@sentry/nextjs';
+      Sentry.init();
+    `;
+
+    const result = valueInjectionLoader.call(loaderThis, userCode);
+
+    expect(result).toMatchSnapshot();
+    expect(result).toMatch(';globalThis["foo"] = "bar";');
+  });
+
+  it('should correctly insert values with directive and multiline block comments and a bunch of whitespace', () => {
+    const userCode = `
+      /*
+        test
+      */
+
+
+
+
+      "use client";
+
+
+
+      import * as Sentry from '@sentry/nextjs';
+      Sentry.init();
+    `;
+
+    const result = valueInjectionLoader.call(loaderThis, userCode);
+
+    expect(result).toMatchSnapshot();
+    expect(result).toMatch(';globalThis["foo"] = "bar";');
+  });
+
+  it('should correctly insert values with a misplaced directive', () => {
+    const userCode = `
+      console.log('This will render the directive useless');
+      "use client";
+
+
+
+      import * as Sentry from '@sentry/nextjs';
+      Sentry.init();
+    `;
+
+    const result = valueInjectionLoader.call(loaderThis, userCode);
+
+    expect(result).toMatchSnapshot();
+    expect(result).toMatch(';globalThis["foo"] = "bar";');
+  });
+});