-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(tanstackstart-react): Auto-copy build file to correct folder #19104
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
d632fe8
d6e225d
3a48e1b
379adc7
7eb02b3
dcf888c
41f210d
882daa1
c9295d4
3cf681e
71014bb
a839ec9
b0456e7
fed3604
a7a6214
3c50a4f
ae72b11
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,104 @@ | ||
| import * as fs from 'fs'; | ||
| import * as path from 'path'; | ||
| import type { Plugin, ResolvedConfig } from 'vite'; | ||
|
|
||
| interface CopyInstrumentationFilePluginOptions { | ||
| instrumentationFilePath?: string; | ||
| serverOutputDir?: string; | ||
| } | ||
|
|
||
| /** | ||
| * Creates a Vite plugin that copies the user's instrumentation file | ||
| * to the server build output directory after the build completes. | ||
| * | ||
| * By default, copies `instrument.server.mjs` from the project root. | ||
| * A custom file path can be provided via `instrumentationFilePath`. | ||
| * | ||
| * The server output directory can be configured via `serverOutputDir`. | ||
| * By default, it will be auto-detected based on the vite plugin being used. | ||
| * | ||
| * For nitro deployments, we use the Nitro Vite environment config to get the server output directory. | ||
| * For cloudflare and netlify deployments, we assume the server output directory is `dist/server`, which is the default output directory for these plugins. | ||
| */ | ||
| export function makeCopyInstrumentationFilePlugin(options?: CopyInstrumentationFilePluginOptions): Plugin { | ||
| let serverOutputDir: string | undefined; | ||
| type RollupOutputDir = { dir?: string }; | ||
| type ViteEnvironments = Record<string, { build?: { rollupOptions?: { output?: RollupOutputDir } } }>; | ||
|
|
||
| return { | ||
| name: 'sentry-tanstackstart-copy-instrumentation-file', | ||
| apply: 'build', | ||
| enforce: 'post', | ||
|
|
||
| configResolved(resolvedConfig: ResolvedConfig) { | ||
| // If user provided serverOutputDir, use it directly and skip auto-detection | ||
| if (options?.serverOutputDir) { | ||
| serverOutputDir = path.resolve(resolvedConfig.root, options.serverOutputDir); | ||
| return; | ||
| } | ||
|
|
||
| const plugins = resolvedConfig.plugins || []; | ||
| const hasPlugin = (name: string): boolean => plugins.some(p => p.name?.includes(name)); | ||
|
|
||
| if (hasPlugin('nitro')) { | ||
| // There seems to be no way to access the nitro instance directly to get the server dir, so we need to access it via the vite environment config. | ||
| // This works because Nitro's Vite bundler sets the rollup output dir to the resolved serverDir: | ||
| // https://github.com/nitrojs/nitro/blob/1954b824597f6ac52fb8b064415cb85d0feda078/src/build/vite/bundler.ts#L35 | ||
| const environments = (resolvedConfig as { environments?: ViteEnvironments }).environments; | ||
| const nitroEnv = environments?.nitro; | ||
| if (nitroEnv) { | ||
| const rollupOutput = nitroEnv.build?.rollupOptions?.output; | ||
| const dir = rollupOutput?.dir; | ||
| if (dir) { | ||
| serverOutputDir = dir; | ||
| } | ||
| } | ||
| } else if (hasPlugin('cloudflare') || hasPlugin('netlify')) { | ||
|
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. l/m: I think we can ignore cloudflare for all purposes here because this approach doesn't work on CF anyway right? I'd rather just keep this code lean. |
||
| // There seems to be no way for users to configure the server output dir for these plugins, so we just assume it's `dist/server`, which is the default output dir. | ||
| serverOutputDir = path.resolve(resolvedConfig.root, 'dist', 'server'); | ||
| } else { | ||
| // eslint-disable-next-line no-console | ||
| console.warn( | ||
|
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. You'll probably need to wrap that in a console wrapper. It could be that Vite also runs this during the dev build. |
||
| '[Sentry] Could not detect nitro, cloudflare, or netlify vite plugin. ' + | ||
| 'The instrument.server.mjs file will not be copied to the build output automatically.', | ||
|
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. l: We could tell them to copy it themselves to their build output folder so it's accessible, wdyt? |
||
| ); | ||
| } | ||
| }, | ||
|
|
||
| async closeBundle() { | ||
| // Auto-detection failed, so we don't copy the instrumentation file. | ||
| if (!serverOutputDir) { | ||
| return; | ||
| } | ||
|
|
||
| const instrumentationFileName = options?.instrumentationFilePath || 'instrument.server.mjs'; | ||
| const instrumentationSource = path.resolve(process.cwd(), instrumentationFileName); | ||
|
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. Bug: The source instrumentation file path is resolved using Suggested FixIn the Prompt for AI AgentDid we get this right? 👍 / 👎 to inform future reviews. |
||
|
|
||
| // Check if the instrumentation file exists. | ||
| try { | ||
| await fs.promises.access(instrumentationSource); | ||
| } catch { | ||
| // eslint-disable-next-line no-console | ||
| console.warn( | ||
| `[Sentry] No ${instrumentationFileName} file found in project root. ` + | ||
| 'The Sentry instrumentation file will not be copied to the build output.', | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| // Copy the instrumentation file to the server output directory. | ||
| const destinationFileName = path.basename(instrumentationFileName); | ||
| const destination = path.resolve(serverOutputDir, destinationFileName); | ||
|
|
||
| try { | ||
| await fs.promises.mkdir(serverOutputDir, { recursive: true }); | ||
| await fs.promises.copyFile(instrumentationSource, destination); | ||
| // eslint-disable-next-line no-console | ||
| console.log(`[Sentry] Copied ${destinationFileName} to ${destination}`); | ||
| } catch (error) { | ||
| // eslint-disable-next-line no-console | ||
| console.warn(`[Sentry] Failed to copy ${destinationFileName} to build output.`, error); | ||
| } | ||
| }, | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import type { BuildTimeOptionsBase } from '@sentry/core'; | ||
| import type { Plugin } from 'vite'; | ||
| import { makeAutoInstrumentMiddlewarePlugin } from './autoInstrumentMiddleware'; | ||
| import { makeCopyInstrumentationFilePlugin } from './copyInstrumentationFile'; | ||
| import { makeAddSentryVitePlugin, makeEnableSourceMapsVitePlugin } from './sourceMaps'; | ||
|
|
||
| /** | ||
|
|
@@ -19,6 +20,29 @@ export interface SentryTanstackStartOptions extends BuildTimeOptionsBase { | |
| * @default true | ||
| */ | ||
| autoInstrumentMiddleware?: boolean; | ||
|
|
||
| /** | ||
| * Path to the instrumentation file to be copied to the server build output directory. | ||
| * | ||
| * Relative paths are resolved from the current working directory. | ||
| * | ||
| * @default 'instrument.server.mjs' | ||
| */ | ||
| instrumentationFilePath?: string; | ||
|
Comment on lines
+24
to
+31
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. Does the SDK look for this file to set up Sentry on the server-side? Q: What if people set a different path here - does the SDK still init the server-side? |
||
|
|
||
| /** | ||
| * Custom server output directory path for the instrumentation file. | ||
| * | ||
| * By default, the plugin auto-detects the output directory: | ||
| * - For Nitro: reads from Vite environment config | ||
| * - For Cloudflare/Netlify: uses `dist/server` | ||
| * | ||
| * Use this option to override the default when your deployment target | ||
| * uses a non-standard output directory. | ||
| * | ||
| * @example 'build/server' | ||
| */ | ||
| serverOutputDir?: string; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -53,6 +77,14 @@ export function sentryTanstackStart(options: SentryTanstackStartOptions = {}): P | |
|
|
||
| const plugins: Plugin[] = [...makeAddSentryVitePlugin(options)]; | ||
|
|
||
| // copy instrumentation file to build output | ||
| plugins.push( | ||
| makeCopyInstrumentationFilePlugin({ | ||
| instrumentationFilePath: options.instrumentationFilePath, | ||
| serverOutputDir: options.serverOutputDir, | ||
| }), | ||
| ); | ||
|
|
||
| // middleware auto-instrumentation | ||
| if (options.autoInstrumentMiddleware !== false) { | ||
| plugins.push(makeAutoInstrumentMiddlewarePlugin({ enabled: true, debug: options.debug })); | ||
|
|
||
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.
L: This is just code style but I'm commenting because I've never seen scoped types in our codebase :D You can just add them at the top of the file.