Skip to content

Commit

Permalink
feat(nuxt): Make dynamic import() wrapping default (#13958)
Browse files Browse the repository at this point in the history 
BREAKING CHANGE: The `--import` flag must not be added anymore. If it is
still set, the server-side will be initialized twice and this leads to
unexpected errors.

---

First merge this:
#13945
Part of this:
#13943

This PR makes it the default to include a rollup plugin that wraps the
server entry file with a dynamic import (`import()`). This is a
replacement for the node `--import` CLI flag.

If you still want to manually add the CLI flag you can use this option
in the `nuxt.config.ts` file:
```js
  sentry: {
    dynamicImportForServerEntry: false,
  }
```

(option name is up for discussion)
  • Loading branch information
@s1gr1d
s1gr1d authored Oct 17, 2024
1 parent 8782af8 commit 6bc37f0
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 84 deletions.
2 changes: 1 addition & 1 deletion dev-packages/e2e-tests/test-applications/nuxt-3/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"dev": "nuxt dev",
"generate": "nuxt generate",
"preview": "nuxt preview",
"start": "node --import ./.output/server/sentry.server.config.mjs .output/server/index.mjs",
"start": "node .output/server/index.mjs",
"clean": "npx nuxi cleanup",
"test": "playwright test",
"test:build": "pnpm install && npx playwright install && pnpm build",
Expand Down
2 changes: 1 addition & 1 deletion dev-packages/e2e-tests/test-applications/nuxt-4/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"dev": "nuxt dev",
"generate": "nuxt generate",
"preview": "nuxt preview",
"start": "node --import ./.output/server/sentry.server.config.mjs .output/server/index.mjs",
"start": "node .output/server/index.mjs",
"clean": "npx nuxi cleanup",
"test": "playwright test",
"test:build": "pnpm install && npx playwright install && pnpm build",
Expand Down
15 changes: 9 additions & 6 deletions packages/nuxt/src/common/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,16 +103,19 @@ export type SentryNuxtModuleOptions = {
debug?: boolean;

/**
* Enabling basic server tracing can be used for environments where modifying the node option `--import` is not possible.
* However, enabling this option only supports limited tracing instrumentation. Only http traces will be collected (but no database-specific traces etc.).
* Wraps the server entry file with a dynamic `import()`. This will make it possible to preload Sentry and register
* necessary hooks before other code runs. (Node docs: https://nodejs.org/api/module.html#enabling)
*
* If this option is `true`, the Sentry SDK will import the Sentry server config at the top of the server entry file to load the SDK on the server.
* If this option is `false`, the Sentry SDK won't wrap the server entry file with `import()`. Not wrapping the
* server entry file will disable Sentry on the server-side. When you set this option to `false`, make sure
* to add the Sentry server config with the node `--import` CLI flag to enable Sentry on the server-side.
*
* **DO NOT** enable this option if you've already added the node option `--import` in your node start script. This would initialize Sentry twice on the server-side and leads to unexpected issues.
* **DO NOT** add the node CLI flag `--import` in your node start script, when `dynamicImportForServerEntry` is set to `true` (default).
* This would initialize Sentry twice on the server-side and this leads to unexpected issues.
*
* @default false
* @default true
*/
experimental_basicServerTracing?: boolean;
dynamicImportForServerEntry?: boolean;

/**
* Options to be passed directly to the Sentry Rollup Plugin (`@sentry/rollup-plugin`) and Sentry Vite Plugin (`@sentry/vite-plugin`) that ship with the Sentry Nuxt SDK.
Expand Down
46 changes: 31 additions & 15 deletions packages/nuxt/src/module.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as path from 'path';
import { addPlugin, addPluginTemplate, addServerPlugin, createResolver, defineNuxtModule } from '@nuxt/kit';
import { consoleSandbox } from '@sentry/utils';
import type { SentryNuxtModuleOptions } from './common/types';
import { addSentryTopImport, addServerConfigToBuild } from './vite/addServerConfig';
import { addDynamicImportEntryFileWrapper, addServerConfigToBuild } from './vite/addServerConfig';
import { setupSourceMaps } from './vite/sourceMaps';
import { findDefaultSdkInitFile } from './vite/utils';

Expand All @@ -17,7 +17,12 @@ export default defineNuxtModule<ModuleOptions>({
},
},
defaults: {},
setup(moduleOptions, nuxt) {
setup(moduleOptionsParam, nuxt) {
const moduleOptions = {
...moduleOptionsParam,
dynamicImportForServerEntry: moduleOptionsParam.dynamicImportForServerEntry !== false, // default: true
};

const moduleDirResolver = createResolver(import.meta.url);
const buildDirResolver = createResolver(nuxt.options.buildDir);

Expand Down Expand Up @@ -48,15 +53,17 @@ export default defineNuxtModule<ModuleOptions>({
const serverConfigFile = findDefaultSdkInitFile('server');

if (serverConfigFile) {
// Inject the server-side Sentry config file with a side effect import
addPluginTemplate({
mode: 'server',
filename: 'sentry-server-config.mjs',
getContents: () =>
`import "${buildDirResolver.resolve(`/${serverConfigFile}`)}"\n` +
'import { defineNuxtPlugin } from "#imports"\n' +
'export default defineNuxtPlugin(() => {})',
});
if (moduleOptions.dynamicImportForServerEntry === false) {
// Inject the server-side Sentry config file with a side effect import
addPluginTemplate({
mode: 'server',
filename: 'sentry-server-config.mjs',
getContents: () =>
`import "${buildDirResolver.resolve(`/${serverConfigFile}`)}"\n` +
'import { defineNuxtPlugin } from "#imports"\n' +
'export default defineNuxtPlugin(() => {})',
});
}

addServerPlugin(moduleDirResolver.resolve('./runtime/plugins/sentry.server'));
}
Expand All @@ -67,11 +74,9 @@ export default defineNuxtModule<ModuleOptions>({

nuxt.hooks.hook('nitro:init', nitro => {
if (serverConfigFile && serverConfigFile.includes('.server.config')) {
addServerConfigToBuild(moduleOptions, nuxt, nitro, serverConfigFile);
if (moduleOptions.dynamicImportForServerEntry === false) {
addServerConfigToBuild(moduleOptions, nuxt, nitro, serverConfigFile);

if (moduleOptions.experimental_basicServerTracing) {
addSentryTopImport(moduleOptions, nitro);
} else {
if (moduleOptions.debug) {
const serverDirResolver = createResolver(nitro.options.output.serverDir);
const serverConfigPath = serverDirResolver.resolve('sentry.server.config.mjs');
Expand All @@ -86,6 +91,17 @@ export default defineNuxtModule<ModuleOptions>({
);
});
}
} else {
addDynamicImportEntryFileWrapper(nitro, serverConfigFile);

if (moduleOptions.debug) {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.log(
'[Sentry] Wrapping the server entry file with a dynamic `import()`, so Sentry can be preloaded before the server initializes.',
);
});
}
}
}
});
Expand Down
57 changes: 3 additions & 54 deletions packages/nuxt/src/vite/addServerConfig.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,54 +74,6 @@ export function addServerConfigToBuild(
});
}

/**
* Adds the Sentry server config import at the top of the server entry file to load the SDK on the server.
* This is necessary for environments where modifying the node option `--import` is not possible.
* However, only limited tracing instrumentation is supported when doing this.
*/
export function addSentryTopImport(moduleOptions: SentryNuxtModuleOptions, nitro: Nitro): void {
nitro.hooks.hook('close', () => {
// other presets ('node-server' or 'vercel') have an index.mjs
const presetsWithServerFile = ['netlify'];
const entryFileName =
typeof nitro.options.rollupConfig?.output.entryFileNames === 'string'
? nitro.options.rollupConfig?.output.entryFileNames
: presetsWithServerFile.includes(nitro.options.preset)
? 'server.mjs'
: 'index.mjs';

const serverDirResolver = createResolver(nitro.options.output.serverDir);
const entryFilePath = serverDirResolver.resolve(entryFileName);

try {
fs.readFile(entryFilePath, 'utf8', (err, data) => {
const updatedContent = `import './${SERVER_CONFIG_FILENAME}.mjs';\n${data}`;

fs.writeFile(entryFilePath, updatedContent, 'utf8', () => {
if (moduleOptions.debug) {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.log(
`[Sentry] Successfully added the Sentry import to the server entry file "\`${entryFilePath}\`"`,
);
});
}
});
});
} catch (err) {
if (moduleOptions.debug) {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.warn(
`[Sentry] An error occurred when trying to add the Sentry import to the server entry file "\`${entryFilePath}\`":`,
err,
);
});
}
}
});
}

/**
* This function modifies the Rollup configuration to include a plugin that wraps the entry file with a dynamic import (`import()`)
* and adds the Sentry server config with the static `import` declaration.
Expand Down Expand Up @@ -187,11 +139,8 @@ function wrapEntryWithDynamicImport(resolvedSentryConfigPath: string): InputPlug
: resolution.id
// Concatenates the query params to mark the file (also attaches names of re-exports - this is needed for serverless functions to re-export the handler)
.concat(SENTRY_WRAPPED_ENTRY)
.concat(
exportedFunctions?.length
? SENTRY_FUNCTIONS_REEXPORT.concat(exportedFunctions.join(',')).concat(QUERY_END_INDICATOR)
: '',
);
.concat(exportedFunctions?.length ? SENTRY_FUNCTIONS_REEXPORT.concat(exportedFunctions.join(',')) : '')
.concat(QUERY_END_INDICATOR);
}
return null;
},
Expand All @@ -211,7 +160,7 @@ function wrapEntryWithDynamicImport(resolvedSentryConfigPath: string): InputPlug
// `import()` can be used for any code that should be run after the hooks are registered (https://nodejs.org/api/module.html#enabling)
`import(${JSON.stringify(entryId)});\n` +
// By importing "import-in-the-middle/hook.mjs", we can make sure this file wil be included, as not all node builders are including files imported with `module.register()`.
"import 'import-in-the-middle/hook.mjs'\n" +
"import 'import-in-the-middle/hook.mjs';\n" +
`${reExportedFunctions}\n`
);
}
Expand Down
7 changes: 4 additions & 3 deletions packages/nuxt/src/vite/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export function extractFunctionReexportQueryParameters(query: string): string[]
return match && match[1]
? match[1]
.split(',')
.filter(param => param !== '' && param !== 'default')
.filter(param => param !== '')
// Sanitize, as code could be injected with another rollup plugin
.map((str: string) => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'))
: [];
Expand All @@ -72,10 +72,11 @@ export function constructFunctionReExport(pathWithQuery: string, entryId: string
return functionNames.reduce(
(functionsCode, currFunctionName) =>
functionsCode.concat(
`export async function ${currFunctionName}(...args) {\n` +
'async function reExport(...args) {\n' +
` const res = await import(${JSON.stringify(entryId)});\n` +
` return res.${currFunctionName}.call(this, ...args);\n` +
'}\n',
'}\n' +
`export { reExport as ${currFunctionName} };\n`,
),
'',
);
Expand Down
29 changes: 25 additions & 4 deletions packages/nuxt/test/vite/utils.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,11 @@ describe('findDefaultSdkInitFile', () => {
describe('removeSentryQueryFromPath', () => {
it('strips the Sentry query part from the path', () => {
const url = `/example/path${SENTRY_WRAPPED_ENTRY}${SENTRY_FUNCTIONS_REEXPORT}foo,${QUERY_END_INDICATOR}`;
const url2 = `/example/path${SENTRY_WRAPPED_ENTRY}${QUERY_END_INDICATOR}`;
const result = removeSentryQueryFromPath(url);
const result2 = removeSentryQueryFromPath(url2);
expect(result).toBe('/example/path');
expect(result2).toBe('/example/path');
});

it('returns the same path if the specific query part is not present', () => {
Expand All @@ -85,7 +88,7 @@ describe('removeSentryQueryFromPath', () => {
describe('extractFunctionReexportQueryParameters', () => {
it.each([
[`${SENTRY_FUNCTIONS_REEXPORT}foo,bar,${QUERY_END_INDICATOR}`, ['foo', 'bar']],
[`${SENTRY_FUNCTIONS_REEXPORT}foo,bar,default${QUERY_END_INDICATOR}`, ['foo', 'bar']],
[`${SENTRY_FUNCTIONS_REEXPORT}foo,bar,default${QUERY_END_INDICATOR}`, ['foo', 'bar', 'default']],
[
`${SENTRY_FUNCTIONS_REEXPORT}foo,a.b*c?d[e]f(g)h|i\\\\j(){hello},${QUERY_END_INDICATOR}`,
['foo', 'a\\.b\\*c\\?d\\[e\\]f\\(g\\)h\\|i\\\\\\\\j\\(\\)\\{hello\\}'],
Expand All @@ -108,18 +111,36 @@ describe('constructFunctionReExport', () => {
const result2 = constructFunctionReExport(query2, entryId);

const expected = `
export async function foo(...args) {
async function reExport(...args) {
const res = await import("./module");
return res.foo.call(this, ...args);
}
export async function bar(...args) {
export { reExport as foo };
async function reExport(...args) {
const res = await import("./module");
return res.bar.call(this, ...args);
}`;
}
export { reExport as bar };
`;
expect(result.trim()).toBe(expected.trim());
expect(result2.trim()).toBe(expected.trim());
});

it('constructs re-export code for a "default" query parameters and entry ID', () => {
const query = `${SENTRY_FUNCTIONS_REEXPORT}default${QUERY_END_INDICATOR}}`;
const entryId = './index';
const result = constructFunctionReExport(query, entryId);

const expected = `
async function reExport(...args) {
const res = await import("./index");
return res.default.call(this, ...args);
}
export { reExport as default };
`;
expect(result.trim()).toBe(expected.trim());
});

it('returns an empty string if the query string is empty', () => {
const query = '';
const entryId = './module';
Expand Down

0 comments on commit 6bc37f0

Please sign in to comment.