-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(tanstackstart-react): Auto-instrument global middleware #18844
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
base: develop
Are you sure you want to change the base?
Changes from all commits
704a781
6cc5784
b4a0d83
b6ddbff
8c6f0fa
207a96a
53e9099
b9c51e9
6eb1c8b
4dcdabf
9aaabd9
bfa238a
086b9f6
05e08f1
bc0acdd
8e60de8
9bbd0b0
283a545
1e736b4
841a311
9ad7a8d
286f624
505c92c
07d5e77
073e352
8ce5358
76c746c
bbf7be4
63a6660
c99e1a1
e2bf4b0
b7d35ad
c6d7624
8142b20
463d7f0
1ed2308
2706bc0
bdb9ac5
a39e88f
00fbbbd
993ba3f
aa7adc5
65a1a3e
7137d00
b5ed7e2
b0c6045
e0c1428
f4d73e0
d2f0801
50f8db0
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 |
|---|---|---|
| @@ -1,9 +1,10 @@ | ||
| import { createStart } from '@tanstack/react-start'; | ||
| import { wrappedGlobalRequestMiddleware, wrappedGlobalFunctionMiddleware } from './middleware'; | ||
| // NOTE: These are NOT wrapped - auto-instrumentation via the Vite plugin will wrap them | ||
| import { globalRequestMiddleware, globalFunctionMiddleware } from './middleware'; | ||
|
|
||
| export const startInstance = createStart(() => { | ||
| return { | ||
| requestMiddleware: [wrappedGlobalRequestMiddleware], | ||
| functionMiddleware: [wrappedGlobalFunctionMiddleware], | ||
| requestMiddleware: [globalRequestMiddleware], | ||
| functionMiddleware: [globalFunctionMiddleware], | ||
| }; | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| import type { Plugin } from 'vite'; | ||
|
|
||
| type AutoInstrumentMiddlewareOptions = { | ||
| enabled?: boolean; | ||
| debug?: boolean; | ||
| }; | ||
|
|
||
| /** | ||
| * A Vite plugin that automatically instruments TanStack Start middlewares | ||
| * by wrapping `requestMiddleware` and `functionMiddleware` arrays in `createStart()`. | ||
| */ | ||
| export function makeAutoInstrumentMiddlewarePlugin(options: AutoInstrumentMiddlewareOptions = {}): Plugin { | ||
| const { enabled = true, debug = false } = options; | ||
|
|
||
| return { | ||
| name: 'sentry-tanstack-middleware-auto-instrument', | ||
| enforce: 'pre', | ||
|
|
||
| transform(code, id) { | ||
| if (!enabled) { | ||
| return null; | ||
| } | ||
|
|
||
| // Skip if not a TS/JS file | ||
| if (!/\.(ts|tsx|js|jsx|mjs|mts)$/.test(id)) { | ||
| return null; | ||
| } | ||
|
|
||
| // Only wrap requestMiddleware and functionMiddleware in createStart() | ||
| if (!code.includes('createStart(')) { | ||
| return null; | ||
| } | ||
|
|
||
| // Skip if the user already did some manual wrapping | ||
| if (code.includes('wrapMiddlewaresWithSentry')) { | ||
| return null; | ||
| } | ||
|
|
||
| let transformed = code; | ||
| let needsImport = false; | ||
| const skippedMiddlewares: string[] = []; | ||
|
|
||
| transformed = transformed.replace( | ||
| /(requestMiddleware|functionMiddleware)\s*:\s*\[([^\]]*)\]/g, | ||
| (match: string, key: string, contents: string) => { | ||
| const objContents = arrayToObjectShorthand(contents); | ||
| if (objContents) { | ||
| needsImport = true; | ||
| if (debug) { | ||
| // eslint-disable-next-line no-console | ||
| console.log(`[Sentry] Auto-wrapping ${key} in ${id}`); | ||
| } | ||
| return `${key}: wrapMiddlewaresWithSentry(${objContents})`; | ||
| } | ||
| // Track middlewares that couldn't be auto-wrapped | ||
| // Skip if we matched whitespace only | ||
| if (contents.trim()) { | ||
|
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: why do we trim the contents here? To make sure it's not an empty file? Just curious
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. Just in case people have weird formatting and we match whitespace only when trying to match the middlewares, unlikely case but shouldn't hurt to have it |
||
| skippedMiddlewares.push(key); | ||
| } | ||
| return match; | ||
| }, | ||
| ); | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
nicohrubec marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // Warn about middlewares that couldn't be auto-wrapped | ||
| if (skippedMiddlewares.length > 0) { | ||
| // eslint-disable-next-line no-console | ||
| console.warn( | ||
| `[Sentry] Could not auto-instrument ${skippedMiddlewares.join(' and ')} in ${id}. ` + | ||
| 'To instrument these middlewares, use wrapMiddlewaresWithSentry() manually. ', | ||
| ); | ||
| } | ||
|
|
||
| // We didn't wrap any middlewares, so we don't need to import the wrapMiddlewaresWithSentry function | ||
| if (!needsImport) { | ||
| return null; | ||
| } | ||
|
|
||
| const sentryImport = "import { wrapMiddlewaresWithSentry } from '@sentry/tanstackstart-react';\n"; | ||
|
|
||
| // Check for 'use server' or 'use client' directives, these need to be before any imports | ||
| const directiveMatch = transformed.match(/^(['"])use (client|server)\1;?\s*\n?/); | ||
| if (directiveMatch) { | ||
| // Insert import after the directive | ||
| const directive = directiveMatch[0]; | ||
| transformed = directive + sentryImport + transformed.slice(directive.length); | ||
| } else { | ||
| transformed = sentryImport + transformed; | ||
| } | ||
|
|
||
| return { code: transformed, map: null }; | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Convert array contents to object shorthand syntax. | ||
| * e.g., "foo, bar, baz" → "{ foo, bar, baz }" | ||
| * | ||
| * Returns null if contents contain non-identifier expressions (function calls, etc.) | ||
| * which cannot be converted to object shorthand. | ||
| */ | ||
| export function arrayToObjectShorthand(contents: string): string | null { | ||
| const items = contents | ||
| .split(',') | ||
| .map(s => s.trim()) | ||
| .filter(Boolean); | ||
|
|
||
| // Only convert if all items are valid identifiers (no complex expressions) | ||
| const allIdentifiers = items.every(item => /^[a-zA-Z_$][a-zA-Z0-9_$]*$/.test(item)); | ||
| if (!allIdentifiers || items.length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| // Deduplicate to avoid invalid syntax like { foo, foo } | ||
| const uniqueItems = [...new Set(items)]; | ||
|
|
||
| return `{ ${uniqueItems.join(', ')} }`; | ||
| } | ||
nicohrubec marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,2 @@ | ||
| export { sentryTanstackStart } from './sentryTanstackStart'; | ||
| export type { SentryTanstackStartOptions } from './sentryTanstackStart'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Middleware regex matches arrays outside createStart scope
Low Severity
The comment on line 29 states the plugin will "Only wrap requestMiddleware and functionMiddleware in createStart()" but the implementation wraps these patterns anywhere in files that merely contain
createStart(. The regex at line 44 matches all occurrences file-wide. If a file defines middleware arrays in separate config objects outside thecreateStart()call, they would also be transformed unexpectedly, potentially wrapping unrelated middleware.