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): Auto-wrap edge-routes and middleware #6746

Merged
merged 10 commits into from
Jan 16, 2023

Conversation

lforst
Copy link
Member

@lforst lforst commented Jan 12, 2023

Ref #6120
Ref #4206

Based on #6771, this PR adds automatic wrapping of Edge routes and Next.js middleware.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 12, 2023

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 19.84 KB (-0.01% 🔽)
@sentry/browser - ES5 CDN Bundle (minified) 61.46 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 18.61 KB (+0.01% 🔺)
@sentry/browser - ES6 CDN Bundle (minified) 55 KB (0%)
@sentry/browser - Webpack (gzipped + minified) 20.37 KB (0%)
@sentry/browser - Webpack (minified) 66.54 KB (0%)
@sentry/react - Webpack (gzipped + minified) 20.4 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified) 47.63 KB (0%)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 26.82 KB (+0.02% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 25.25 KB (-0.01% 🔽)
@sentry/replay ES6 CDN Bundle (gzipped + minified) 43.35 KB (-0.84% 🔽)
@sentry/replay - Webpack (gzipped + minified) 38.58 KB (-0.97% 🔽)

@lforst lforst force-pushed the lforst-edge-auto-wrapping branch from 760df35 to c31d07f Compare January 12, 2023 16:47
@lforst lforst changed the base branch from lforst-nextjs-sdk-multiplexer to master January 13, 2023 16:38
@lforst lforst force-pushed the lforst-edge-auto-wrapping branch from 1eb3183 to 416a7c8 Compare January 13, 2023 16:52
@lforst lforst changed the base branch from master to lforst-edge-wrappers January 13, 2023 16:53
@lforst lforst requested review from mydea and AbhiPrasad January 16, 2023 08:25
@lforst lforst marked this pull request as ready for review January 16, 2023 08:25
* This file is a template for the code which will be substituted when our webpack loader handles API files in the
* `pages/` directory.
*
* We use `__RESOURCE_PATH__` as a placeholder for the path to the file being wrapped. Because it's not a real package,
* We use `__SENTRY_WRAPPING_TARGET__` as a placeholder for the path to the file being wrapped. Because it's not a real package,
Copy link
Member

Choose a reason for hiding this comment

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

l: should we call this __SENTRY_WRAPPING_TARGET_FILE__ to be a little more clear?

@@ -99,23 +99,61 @@ export function constructWebpackConfigFunction(

if (isServer) {
if (userSentryOptions.autoInstrumentServerFunctions !== false) {
const pagesDir = newConfig.resolve?.alias?.['private-next-pages'] as string;
Copy link
Member

Choose a reason for hiding this comment

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

why this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

We looked at this really obscure field with a typecast here, and I am not sure if it is even part of the Next.js public API. Behavior around the pages and pages/src directory is pretty well defined here and it felt safer in the long run to just do the lookup ourselves. I guess it is not really required to be in this PR but snuck it in as a little cleanup.

Base automatically changed from lforst-edge-wrappers to master January 16, 2023 11:10
@lforst lforst merged commit 4d5f13e into master Jan 16, 2023
@lforst lforst deleted the lforst-edge-auto-wrapping branch January 16, 2023 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants