-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(nuxt): Add server config to root folder #13583
Changes from 4 commits
ea95e37
7671bd9
ae6d51c
d21dac2
16ef5ac
5a72b0b
2be97af
e6a78fb
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,64 @@ | ||
import * as fs from 'fs'; | ||
import * as path from 'path'; | ||
import { createResolver } from '@nuxt/kit'; | ||
import type { Nuxt } from '@nuxt/schema'; | ||
import type { SentryNuxtModuleOptions } from '../common/types'; | ||
|
||
/** | ||
* Adds the `server.config.ts` file as `instrument-sentry.mjs` to the `.output` directory to be able to reference this file in the node --import option. | ||
* | ||
* 1. Adding the file as a rollup import, so it is included in the build (automatically transpiles the file). | ||
* 2. Copying the file to the `.output` directory after the build process is finished. | ||
*/ | ||
export function addServerConfigToBuild( | ||
moduleOptions: SentryNuxtModuleOptions, | ||
nuxt: Nuxt, | ||
serverConfigFile: string, | ||
): void { | ||
if (moduleOptions.debug) { | ||
// eslint-disable-next-line no-console | ||
console.log( | ||
'[Sentry] Using your `sentry.server.config` file for the server-side Sentry configuration. In case you have a `public/instrument.server` file, it will be ignored.', | ||
); | ||
} | ||
|
||
nuxt.hook('vite:extendConfig', async (viteInlineConfig, _env) => { | ||
if ( | ||
typeof viteInlineConfig?.build?.rollupOptions?.input === 'object' && | ||
'server' in viteInlineConfig.build.rollupOptions.input | ||
) { | ||
// Create a rollup entry for the server config to add it as `instrument-sentry.mjs` to the build | ||
(viteInlineConfig.build.rollupOptions.input as { [entryName: string]: string })['instrument-sentry'] = | ||
createResolver(nuxt.options.srcDir).resolve(`/${serverConfigFile}`); | ||
} | ||
|
||
/** | ||
* When the build process is finished, copy the `sentry.server.config` file to the `.output` directory. | ||
* This is necessary because we need to reference this file path in the node --import option. | ||
*/ | ||
nuxt.hook('close', async () => { | ||
const source = path.resolve('.nuxt/dist/server/instrument-sentry.mjs'); | ||
const destination = path.resolve('.output/server/instrument-sentry.mjs'); | ||
|
||
try { | ||
await fs.promises.access(source, fs.constants.F_OK); | ||
await fs.promises.copyFile(source, destination); | ||
|
||
if (moduleOptions.debug) { | ||
// eslint-disable-next-line no-console | ||
console.log( | ||
'[Sentry] Successfully added the content of the `sentry.server.config` file as `instrument-sentry.mjs` to the `.output/server` directory', | ||
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: Maybe we can use |
||
); | ||
} | ||
} catch (error) { | ||
if (moduleOptions.debug) { | ||
// eslint-disable-next-line no-console | ||
console.warn( | ||
'[Sentry] An error occurred when trying to add the `sentry.server.config` file to the `.output` directory', | ||
error, | ||
); | ||
} | ||
} | ||
}); | ||
}); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import * as fs from 'fs'; | ||
import * as path from 'path'; | ||
|
||
/** | ||
* Find the default SDK init file for the given type (client or server). | ||
* The sentry.server.config file is prioritized over the instrument.server file. | ||
*/ | ||
export function findDefaultSdkInitFile(type: 'server' | 'client'): string | undefined { | ||
const possibleFileExtensions = ['ts', 'js', 'mjs', 'cjs', 'mts', 'cts']; | ||
const cwd = process.cwd(); | ||
|
||
const filePaths: string[] = []; | ||
if (type === 'server') { | ||
for (const ext of possibleFileExtensions) { | ||
// order is important here - we want to prioritize the server.config file | ||
filePaths.push(path.join(cwd, `sentry.${type}.config.${ext}`)); | ||
filePaths.push(path.join(cwd, 'public', `instrument.${type}.${ext}`)); | ||
} | ||
} else { | ||
for (const ext of possibleFileExtensions) { | ||
filePaths.push(path.join(cwd, `sentry.${type}.config.${ext}`)); | ||
} | ||
} | ||
|
||
const filePath = filePaths.find(filename => fs.existsSync(filename)); | ||
|
||
return filePath ? path.basename(filePath) : undefined; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
import * as fs from 'fs'; | ||
import { afterEach, describe, expect, it, vi } from 'vitest'; | ||
import { findDefaultSdkInitFile } from '../../src/vite/utils'; | ||
|
||
vi.mock('fs'); | ||
|
||
describe('findDefaultSdkInitFile', () => { | ||
afterEach(() => { | ||
vi.clearAllMocks(); | ||
}); | ||
|
||
it('should return the server file if it exists', () => { | ||
vi.spyOn(fs, 'existsSync').mockImplementation(filePath => { | ||
return !(filePath instanceof URL) && filePath.includes('sentry.server.config.js'); | ||
}); | ||
|
||
const result = findDefaultSdkInitFile('server'); | ||
expect(result).toBe('sentry.server.config.js'); | ||
}); | ||
|
||
it('should return the client file if it exists', () => { | ||
vi.spyOn(fs, 'existsSync').mockImplementation(filePath => { | ||
return !(filePath instanceof URL) && filePath.includes('sentry.client.config.js'); | ||
}); | ||
|
||
const result = findDefaultSdkInitFile('client'); | ||
expect(result).toBe('sentry.client.config.js'); | ||
}); | ||
|
||
it('should return undefined if no file exists', () => { | ||
vi.spyOn(fs, 'existsSync').mockReturnValue(false); | ||
|
||
const result = findDefaultSdkInitFile('server'); | ||
expect(result).toBeUndefined(); | ||
}); | ||
|
||
it('should return the server config file if server.config and instrument exist', () => { | ||
vi.spyOn(fs, 'existsSync').mockImplementation(filePath => { | ||
return ( | ||
!(filePath instanceof URL) && | ||
(filePath.includes('sentry.server.config.js') || filePath.includes('instrument.server.js')) | ||
); | ||
}); | ||
|
||
const result = findDefaultSdkInitFile('server'); | ||
expect(result).toBe('sentry.server.config.js'); | ||
}); | ||
|
||
it('should return the server file with .ts extension if it exists', () => { | ||
vi.spyOn(fs, 'existsSync').mockImplementation(filePath => { | ||
return !(filePath instanceof URL) && filePath.includes('sentry.server.config.ts'); | ||
}); | ||
|
||
const result = findDefaultSdkInitFile('server'); | ||
expect(result).toBe('sentry.server.config.ts'); | ||
}); | ||
|
||
it('should return the client file with .mjs extension if it exists', () => { | ||
vi.spyOn(fs, 'existsSync').mockImplementation(filePath => { | ||
return !(filePath instanceof URL) && filePath.includes('sentry.client.config.mjs'); | ||
}); | ||
|
||
const result = findDefaultSdkInitFile('client'); | ||
expect(result).toBe('sentry.client.config.mjs'); | ||
}); | ||
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: For some test extra credit (feel free to disregard) - we could use an |
||
|
||
it('should return undefined if no file with specified extensions exists', () => { | ||
vi.spyOn(fs, 'existsSync').mockReturnValue(false); | ||
|
||
const result = findDefaultSdkInitFile('server'); | ||
expect(result).toBeUndefined(); | ||
}); | ||
}); |
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.
This reads a bit confusing, which of the two files are being ignored?
I think we could check here for the existence of the file and log explicitly if that file is getting ignored, wdyt?
Also, we are ignoring the public file here but users will still potentially
--import
it. This log makes it sound like it's completely ignored.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.
I think this log should be split up.
1st part (public): can we log a warning if we detect the file in
public
instead of the first part of this message? Otherwise, I don't think we need to bother users with it, right?To the second part: Can we instead print the exact import command? Something like
"Make sure to add
--import ./.output/sentry.server.config.mjs
to yourNODE_OPTIONS
env variable"