Skip to content
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(nextjs): Add auto-wrapping for server components #6953

Merged
merged 27 commits into from
Feb 2, 2023
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
efa8c02
Add simple wrapper
lforst Jan 27, 2023
d639de8
Refactor initialization of loaders a bit
lforst Jan 27, 2023
9c24ebb
Fix build
lforst Jan 27, 2023
f0f6d4a
Call wrapping loader for server components
lforst Jan 27, 2023
6b3f13a
Auto wrap server components
lforst Jan 27, 2023
bd4d8c8
Whoopsie
lforst Jan 27, 2023
57b44df
🤯
lforst Jan 27, 2023
c588f78
Wrap under the assumption that server components may return promises …
lforst Jan 27, 2023
1f398ae
Add escape hatch
lforst Jan 27, 2023
7624607
Add integration test
lforst Jan 27, 2023
2c89413
Wait wtf
lforst Jan 27, 2023
ec9f668
Testing the integration tests
lforst Jan 27, 2023
fdf9300
Run server tests for app dir
lforst Jan 27, 2023
5aa6cf5
Fix
lforst Jan 27, 2023
cb0e21f
fix(nextjs): Don't modify require calls
lforst Jan 30, 2023
09377e1
Merge remote-tracking branch 'origin/master' into lforst-add-app-dire…
lforst Jan 30, 2023
9df4971
Merge branch 'lforst-fix-cjs-imports' into lforst-add-app-directory-c…
lforst Jan 30, 2023
82fae38
...
lforst Jan 30, 2023
4e204a5
Merge remote-tracking branch 'origin/develop' into lforst-add-app-dir…
lforst Feb 1, 2023
e94ac42
rm rf tests because they're broken
lforst Feb 1, 2023
b24868a
Merge branch 'develop' into lforst-add-app-directory-component-wrappers
lforst Feb 1, 2023
2948f0b
Use proxy
lforst Feb 1, 2023
6631195
Merge branch 'develop' into lforst-add-app-directory-component-wrappers
lforst Feb 2, 2023
1e34bda
Merge branch 'develop' into lforst-add-app-directory-component-wrappers
lforst Feb 2, 2023
990ff58
Remove leftover
lforst Feb 2, 2023
9401d66
Add tests
lforst Feb 2, 2023
0674a9d
Fix typo
lforst Feb 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/nextjs/rollup.npm.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export default [
'src/client/index.ts',
'src/server/index.ts',
'src/edge/index.ts',
'src/config/webpack.ts',
'src/config/index.ts',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an oversight from before

],

// prevent this internal nextjs code from ending up in our built package (this doesn't happen automatially because
Expand All @@ -25,6 +25,7 @@ export default [
'src/config/templates/pageWrapperTemplate.ts',
'src/config/templates/apiWrapperTemplate.ts',
'src/config/templates/middlewareWrapperTemplate.ts',
'src/config/templates/serverComponentWrapperTemplate.ts',
],

packageSpecificConfig: {
Expand Down
2 changes: 2 additions & 0 deletions packages/nextjs/src/client/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,5 @@ export {
withSentryServerSideErrorGetInitialProps,
wrapErrorGetInitialPropsWithSentry,
} from './wrapErrorGetInitialPropsWithSentry';

export { wrapAppDirComponentWithSentry } from './wrapAppDirComponentWithSentry';
7 changes: 7 additions & 0 deletions packages/nextjs/src/client/wrapAppDirComponentWithSentry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/**
* Currently just a pass-through to provide isomorphism for the client. May be used in the future to add instrumentation
* for client components.
*/
export function wrapAppDirComponentWithSentry(wrappingTarget: any): any {
return wrappingTarget;
}
112 changes: 80 additions & 32 deletions packages/nextjs/src/config/loaders/wrappingLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ const pageWrapperTemplateCode = fs.readFileSync(pageWrapperTemplatePath, { encod
const middlewareWrapperTemplatePath = path.resolve(__dirname, '..', 'templates', 'middlewareWrapperTemplate.js');
const middlewareWrapperTemplateCode = fs.readFileSync(middlewareWrapperTemplatePath, { encoding: 'utf8' });

const serverComponentWrapperTemplatePath = path.resolve(
__dirname,
'..',
'templates',
'serverComponentWrapperTemplate.js',
);
const serverComponentWrapperTemplateCode = fs.readFileSync(serverComponentWrapperTemplatePath, { encoding: 'utf8' });

// Just a simple placeholder to make referencing module consistent
const SENTRY_WRAPPER_MODULE_NAME = 'sentry-wrapper-module';

Expand All @@ -23,8 +31,10 @@ const WRAPPING_TARGET_MODULE_NAME = '__SENTRY_WRAPPING_TARGET_FILE__.cjs';

type LoaderOptions = {
pagesDir: string;
appDir: string;
pageExtensionRegex: string;
excludeServerRoutes: Array<RegExp | string>;
wrappingTargetKind: 'page' | 'api-route' | 'middleware' | 'page-server-component';
};

/**
Expand All @@ -36,52 +46,91 @@ export default function wrappingLoader(
this: LoaderThis<LoaderOptions>,
userCode: string,
userModuleSourceMap: any,
): void | string {
): void {
// We know one or the other will be defined, depending on the version of webpack being used
const {
pagesDir,
appDir,
pageExtensionRegex,
excludeServerRoutes = [],
wrappingTargetKind,
} = 'getOptions' in this ? this.getOptions() : this.query;

this.async();

// Get the parameterized route name from this page's filepath
const parameterizedRoute = path
// Get the path of the file insde of the pages directory
.relative(pagesDir, this.resourcePath)
// Add a slash at the beginning
.replace(/(.*)/, '/$1')
// Pull off the file extension
.replace(new RegExp(`\\.(${pageExtensionRegex})`), '')
// Any page file named `index` corresponds to root of the directory its in, URL-wise, so turn `/xyz/index` into
// just `/xyz`
.replace(/\/index$/, '')
// In case all of the above have left us with an empty string (which will happen if we're dealing with the
// homepage), sub back in the root route
.replace(/^$/, '/');

// Skip explicitly-ignored pages
if (stringMatchesSomePattern(parameterizedRoute, excludeServerRoutes, true)) {
this.callback(null, userCode, userModuleSourceMap);
return;
}
let templateCode: string;

const middlewareJsPath = path.join(pagesDir, '..', 'middleware.js');
const middlewareTsPath = path.join(pagesDir, '..', 'middleware.ts');
if (wrappingTargetKind === 'page' || wrappingTargetKind === 'api-route') {
// Get the parameterized route name from this page's filepath
const parameterizedPagesRoute = path.posix
.normalize(
path
// Get the path of the file insde of the pages directory
.relative(pagesDir, this.resourcePath),
)
// Add a slash at the beginning
.replace(/(.*)/, '/$1')
// Pull off the file extension
.replace(new RegExp(`\\.(${pageExtensionRegex})`), '')
// Any page file named `index` corresponds to root of the directory its in, URL-wise, so turn `/xyz/index` into
// just `/xyz`
.replace(/\/index$/, '')
// In case all of the above have left us with an empty string (which will happen if we're dealing with the
// homepage), sub back in the root route
.replace(/^$/, '/');

let templateCode: string;
if (parameterizedRoute.startsWith('/api')) {
templateCode = apiWrapperTemplateCode;
} else if (this.resourcePath === middlewareJsPath || this.resourcePath === middlewareTsPath) {
// Skip explicitly-ignored pages
if (stringMatchesSomePattern(parameterizedPagesRoute, excludeServerRoutes, true)) {
this.callback(null, userCode, userModuleSourceMap);
return;
}

if (wrappingTargetKind === 'page') {
templateCode = pageWrapperTemplateCode;
} else if (wrappingTargetKind === 'api-route') {
templateCode = apiWrapperTemplateCode;
} else {
throw new Error(`Invariant: Could not get template code of unknown kind "${wrappingTargetKind}"`);
}

// Inject the route and the path to the file we're wrapping into the template
templateCode = templateCode.replace(/__ROUTE__/g, parameterizedPagesRoute.replace(/\\/g, '\\\\'));
} else if (wrappingTargetKind === 'page-server-component') {
// Get the parameterized route name from this page's filepath
const parameterizedPagesRoute = path.posix
.normalize(path.relative(appDir, this.resourcePath))
// Add a slash at the beginning
.replace(/(.*)/, '/$1')
// Pull off the file name
.replace(/\/page\.(js|jsx|tsx)$/, '')
// Remove routing groups: https://beta.nextjs.org/docs/routing/defining-routes#example-creating-multiple-root-layouts
.replace(/\/(\(.*?\)\/)+/g, '/')
// In case all of the above have left us with an empty string (which will happen if we're dealing with the
// homepage), sub back in the root route
.replace(/^$/, '/');

// Skip explicitly-ignored pages
if (stringMatchesSomePattern(parameterizedPagesRoute, excludeServerRoutes, true)) {
this.callback(null, userCode, userModuleSourceMap);
return;
}

// The following string is what Next.js injects in order to mark client components:
// https://github.com/vercel/next.js/blob/295f9da393f7d5a49b0c2e15a2f46448dbdc3895/packages/next/build/analysis/get-page-static-info.ts#L37
// https://github.com/vercel/next.js/blob/a1c15d84d906a8adf1667332a3f0732be615afa0/packages/next-swc/crates/core/src/react_server_components.rs#L247
// We do not want to wrap client components
if (userCode.includes('/* __next_internal_client_entry_do_not_use__ */')) {
this.callback(null, userCode, userModuleSourceMap);
return;
}

templateCode = serverComponentWrapperTemplateCode;
} else if (wrappingTargetKind === 'middleware') {
templateCode = middlewareWrapperTemplateCode;
} else {
templateCode = pageWrapperTemplateCode;
throw new Error(`Invariant: Could not get template code of unknown kind "${wrappingTargetKind}"`);
}

// Inject the route and the path to the file we're wrapping into the template
templateCode = templateCode.replace(/__ROUTE__/g, parameterizedRoute.replace(/\\/g, '\\\\'));

// Replace the import path of the wrapping target in the template with a path that the `wrapUserCode` function will understand.
templateCode = templateCode.replace(/__SENTRY_WRAPPING_TARGET_FILE__/g, WRAPPING_TARGET_MODULE_NAME);

Expand All @@ -97,7 +146,6 @@ export default function wrappingLoader(
`[@sentry/nextjs] Could not instrument ${this.resourcePath}. An error occurred while auto-wrapping:\n${err}`,
);
this.callback(null, userCode, userModuleSourceMap);
return;
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* This file is a template for the code which will be substituted when our webpack loader handles non-API files in the
* `pages/` directory.
*
* We use `__SENTRY_WRAPPING_TARGET_FILE__` as a placeholder for the path to the file being wrapped. Because it's not a real package,
* this causes both TS and ESLint to complain, hence the pragma comments below.
*/

// @ts-ignore See above
// eslint-disable-next-line import/no-unresolved
import * as wrapee from '__SENTRY_WRAPPING_TARGET_FILE__';
// eslint-disable-next-line import/no-extraneous-dependencies
import * as Sentry from '@sentry/nextjs';

type ServerComponentModule = {
default: unknown;
};

const serverComponentModule = wrapee as ServerComponentModule;

const serverComponent = serverComponentModule.default;

let wrappedServerComponent;
if (typeof serverComponent === 'function') {
wrappedServerComponent = Sentry.wrapAppDirComponentWithSentry(serverComponent);
} else {
wrappedServerComponent = serverComponent;
}

// Re-export anything exported by the page module we're wrapping. When processing this code, Rollup is smart enough to
// not include anything whose name matchs something we've explicitly exported above.
// @ts-ignore See above
// eslint-disable-next-line import/no-unresolved
export * from '__SENTRY_WRAPPING_TARGET_FILE__';

export default wrappedServerComponent;
7 changes: 6 additions & 1 deletion packages/nextjs/src/config/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,14 @@ export type UserSentryOptions = {
*/
autoInstrumentMiddleware?: boolean;

/**
* Automatically instrument components in the `app` directory with error monitoring. Defaults to `true`.
*/
autoInstrumentAppDirectory?: boolean;

/**
* Exclude certain serverside API routes or pages from being instrumented with Sentry. This option takes an array of
* strings or regular expressions.
* strings or regular expressions. This options also affects pages in the `app` directory.
*
* NOTE: Pages should be specified as routes (`/animals` or `/api/animals/[animalType]/habitat`), not filepaths
* (`pages/animals/index.js` or `.\src\pages\api\animals\[animalType]\habitat.tsx`), and strings must be be a full,
Expand Down