-
-
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
Open
nicohrubec
wants to merge
17
commits into
develop
Choose a base branch
from
nh/auto-copy-build-file
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
d632fe8
feat(tanstackstart-react): Auto-copy build file to correct folder
nicohrubec d6e225d
detect by plugin name
nicohrubec 3a48e1b
desloüg
nicohrubec 379adc7
Add warn message if no instrument file found
nicohrubec 7eb02b3
instrument file path configurable
nicohrubec dcf888c
yf
nicohrubec 41f210d
Fix nitro
nicohrubec 882daa1
be more explicit with the type
nicohrubec c9295d4
comment
nicohrubec 3cf681e
add escape hatch
nicohrubec 71014bb
simple logs
nicohrubec a839ec9
clean up tests
nicohrubec b0456e7
cleanup
nicohrubec fed3604
cleanup tests
nicohrubec a7a6214
improve tests
nicohrubec 3c50a4f
changelog
nicohrubec ae72b11
changelog
nicohrubec File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
104 changes: 104 additions & 0 deletions
104
packages/tanstackstart-react/src/vite/copyInstrumentationFile.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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')) { | ||
| // 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( | ||
| '[Sentry] Could not detect nitro, cloudflare, or netlify vite plugin. ' + | ||
| 'The instrument.server.mjs file will not be copied to the build output automatically.', | ||
| ); | ||
| } | ||
| }, | ||
|
|
||
| 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); | ||
|
|
||
| // 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); | ||
| } | ||
| }, | ||
| }; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Bug: The source instrumentation file path is resolved using
process.cwd(), while the destination usesresolvedConfig.root. This will fail when these paths differ, like in monorepos.Severity: MEDIUM
Suggested Fix
In the
configResolvedhook, storeresolvedConfig.rootin a closure-scoped variable. Then, in thecloseBundlehook, use this stored variable instead ofprocess.cwd()to resolve the source instrumentation file's path. This ensures both source and destination paths are resolved relative to the same Vite project root.Prompt for AI Agent
Did we get this right? 👍 / 👎 to inform future reviews.