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

ref(nextjs): Use @sentry/vercel-edge in nextjs package #9053

Merged
merged 3 commits into from
Sep 20, 2023

Conversation

AbhiPrasad
Copy link
Member

@AbhiPrasad AbhiPrasad commented Sep 19, 2023

ref #8087
Dependent on #9051 merging in

@lforst lforst self-requested a review September 20, 2023 08:02
Copy link
Member

@lforst lforst left a comment

Choose a reason for hiding this comment

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

lgtm. Not entirely relevant to the PR but I wonder if we should actually publish the vercel edge package. We could just inline it instead.

Shouldn't block this though.

Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

Also LGTM from my PoV. Adjusted e2e and integration tests.
A slight change that shouldn't have much effect on in-product features is that the event contexts.runtime changes from edge to vercel-edge. Just noting it here for future readers.

RE publishing vs inlining: The advantage of publishing the package is that other, non-NextJS users can try the package. We previously marked it as alpha state so if we need to break things, we can.

@Lms24 Lms24 merged commit b372f8e into develop Sep 20, 2023
@Lms24 Lms24 deleted the abhi-nextjs-vercel-edge branch September 20, 2023 08:56
@AbhiPrasad AbhiPrasad self-assigned this Sep 20, 2023
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.

3 participants