Skip to content

ref(nextjs): Update webpack-plugin and change how cli binary is detected #4984

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

Closed
wants to merge 1 commit into from

Conversation

kamilogorek
Copy link
Contributor

It ain't nice, but it works. More details in the code comment.

@AbhiPrasad
Copy link
Member

We just cut a release unfortunately, and I don't think we'll have another one before v7.

@smeubank should be reasonable for us to just add this to v7 right? Another incentive for folks to upgrade!

@smeubank
Copy link
Member

once we are confident with the fix let's communicate it to vercel, and work with them that it is tested as soon as we have a beta version ready to test with at least an example NextJs application. Then we can ask the folks from the issue to help us with testing it as well

@AbhiPrasad AbhiPrasad added this to the 7.0.0 milestone Apr 26, 2022
@kamilogorek
Copy link
Contributor Author

Confirmed on my local repo that it works correctly. Still need to verify on Vercel, but we need beta (or normal) release for this.

@AbhiPrasad
Copy link
Member

Can we use https://github.com/getsentry/sentry-javascript/blob/master/packages/nextjs/vercel/install-sentry-from-branch.sh to test?

@kamilogorek
Copy link
Contributor Author

Pre

Large Dependencies                    Uncompressed size  Compressed size
--
16:23:25.878 | node_modules/@sentry/cli                        18.1 MB          6.83 MB
16:23:25.878 | node_modules/next/dist                          31.9 MB          6.33 MB
16:23:25.878 | node_modules/caniuse-lite/data                   775 kB           294 kB
16:23:25.878 | node_modules/react-dom/cjs                      1.18 MB           289 kB
16:23:25.878 | node_modules/encoding/node_modules               329 kB           179 kB
16:23:25.878 | node_modules/iconv-lite/encodings                303 kB           172 kB
16:23:25.878 | node_modules/@next/react-dev-overlay             476 kB           139 kB
16:23:25.878 |  
16:23:25.878 | All dependencies                                58.7 MB          15.8 MB

Post

Large Dependencies                    Uncompressed size  Compressed size
--
16:41:14.458 | node_modules/next/dist                          31.9 MB          6.33 MB
16:41:14.458 | node_modules/caniuse-lite/data                   775 kB           294 kB
16:41:14.458 | node_modules/react-dom/cjs                      1.18 MB           289 kB
16:41:14.458 | node_modules/@sentry/nextjs                      887 kB           276 kB
16:41:14.458 | node_modules/encoding/node_modules               329 kB           179 kB
16:41:14.458 | node_modules/iconv-lite/encodings                303 kB           172 kB
16:41:14.458 | node_modules/@next/react-dev-overlay             476 kB           139 kB
16:41:14.458 |  
16:41:14.458 | All dependencies                                40.8 MB          9.04 MB

Copy link
Member

@lobsterkatie lobsterkatie left a comment

Choose a reason for hiding this comment

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

Mashing approve while holding my nose...

(To be clear, that's about the need for the workaround in the first place. I appreciate you fixing this!)

@AbhiPrasad
Copy link
Member

@kamilogorek I'm gonna take this over and rebase onto 7.x. Let's move the change there!

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.

4 participants