Skip to content

Comments

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

Closed
kamilogorek wants to merge 1 commit intomasterfrom
vercel-cli
Closed

ref(nextjs): Update webpack-plugin and change how cli binary is detected#4984
kamilogorek wants to merge 1 commit intomasterfrom
vercel-cli

Conversation

@kamilogorek
Copy link
Contributor

@kamilogorek kamilogorek commented Apr 26, 2022

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

@AbhiPrasad
Copy link
Contributor

AbhiPrasad commented Apr 26, 2022

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
Contributor

smeubank commented Apr 26, 2022

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

kamilogorek commented Apr 26, 2022

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
Contributor

AbhiPrasad commented Apr 26, 2022

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

kamilogorek commented Apr 26, 2022

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

lobsterkatie
lobsterkatie approved these changes Apr 26, 2022
View reviewed changes
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
Contributor

AbhiPrasad commented Apr 26, 2022

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

@AbhiPrasad AbhiPrasad self-assigned this Apr 26, 2022
@AbhiPrasad AbhiPrasad mentioned this pull request Apr 26, 2022
Merged
@kamilogorek kamilogorek deleted the vercel-cli branch April 27, 2022 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lobsterkatie lobsterkatie lobsterkatie approved these changes

@AbhiPrasad AbhiPrasad Awaiting requested review from AbhiPrasad

Assignees

@AbhiPrasad AbhiPrasad

Labels

None yet

Projects

None yet

Milestone

7.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@kamilogorek @AbhiPrasad @smeubank @lobsterkatie