Skip to content

fix(nextjs): Add next as a peer dependency #3504

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

Merged
merged 1 commit into from
May 6, 2021

Conversation

lobsterkatie
Copy link
Member

Since there are now a few different places in our code where we import things from next, it should be a dependency. That said, anyone using the SDK presumably already has next installed, so this PR makes it a peer dependency rather than a regular dependency.

Fixes #3490.

@lobsterkatie lobsterkatie requested a review from kamilogorek as a code owner May 5, 2021 17:49
@github-actions
Copy link
Contributor

github-actions bot commented May 5, 2021

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 20.63 KB (+0.01% 🔺)
@sentry/browser - Webpack 21.5 KB (0%)
@sentry/react - Webpack 21.53 KB (0%)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 27.92 KB (+0.01% 🔺)

@HazAT
Copy link
Member

HazAT commented May 6, 2021

Want to add that we need next 10.0.8 because they fixed a bug there which our SDK relies on.
I don't have a link to the issue but a customer reported it.

@HazAT HazAT merged commit fc112b1 into master May 6, 2021
@HazAT HazAT deleted the kmclb-make-next-a-peer-dependency branch May 6, 2021 07:37
@lobsterkatie
Copy link
Member Author

Want to add that we need next 10.0.8 because they fixed a bug there which our SDK relies on.
I don't have a link to the issue but a customer reported it.

They reported it through one of our sales engineers, I believe, who passed the feedback onto us in slack:

I've started experimenting with the NextJS integration.

I noticed on https://docs.sentry.io/platforms/javascript/guides/nextjs/, it states:

Currently, the minimum Next.js supported version is 10.0.0.

Our app is currently using 10.0.3, but I was running into an issue when I added withSentryConfig to my next.config.js file. From what I could tell, the issue was due to this instrumentServer call:

Which in turn creates a nextServer:

const nextServerPrototype = Object.getPrototypeOf(createNextServer({}));

Which tries to load the next.config.js file and results in this:

`warn  - Detected next.config.js, no exported configuration found. https://err.sh/vercel/next.js/empty-configuration`

I noticed that there were some config loading changes in 10.0.8, namely: vercel/next.js#22578 and so I tried upgrading and that seemed to resolve the issue.

Wanted to give you the heads up so your team can confirm and update that note in the docs about the minimum required version of NextJS.

The issue is that before 10.0.8, config loading was sync rather than async.

H/t to the original reporter!

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.

@sentry/nextjs: next incorrectly a devDependency rather than dependency
2 participants