-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat(nextjs) Update quickstart and manual setup #15757
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) Update quickstart and manual setup #15757
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Bundle ReportChanges will increase total bundle size by 108.94kB (0.31%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: sentry-docs-client-array-pushAssets Changed:
view changes for bundle: sentry-docs-server-cjsAssets Changed:
|
…ide-nextjs' of github.com:getsentry/sentry-docs into sergiydybskiy/devex-149-net-new-full-getting-started-guide-nextjs
|
Hi @sergical! Wizard
Manual SetupLike i wrote earlier, great job taking on this beast of a guide :D A few notes:
Pages Router
Webpack
|
docs/platforms/javascript/guides/nextjs/manual-setup/webpack-setup.mdx
Outdated
Show resolved
Hide resolved
coolguyzone
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
…etup.mdx Co-authored-by: Alex Krawiec <alex.krawiec@sentry.io>
inventarSarah
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates -- I really like this version! 💫
This is more a curiosity question than feedback: I was wondering what the reasoning was behind creating an almost complete Pages Router guide while leaving out things like the onboarding options.
| 3. Click **"Throw Sample Error"** | ||
|
|
||
| Don't forget to explore the example files' code in your project to understand what's happening after your button click. | ||
| 4. Check your data in Sentry: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hierarchy is a bit off (the content after ":" is not part of the list) -- I'd say either move "check your data" outside the list, maybe as a subheading.
Alternatively, we could include the content below in the list, but I don't think that would look good :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk i made it better or worse 😅
|
|
||
| <OnboardingOption optionId="session-replay"> | ||
|
|
||
| ## Session Replay |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the user doesn't enable this feature, this step is missing and the guide is jumping from step 3 to step 5 (tracing)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, restructured to have the features all be part of the verify step, lmk what you think!
| --- | ||
| title: "Manual Setup" | ||
| sidebar_order: 1 | ||
| description: "Learn how to set up and configure Sentry in your Next.js application, capture your first errors, logs and traces and view them in Sentry" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| description: "Learn how to set up and configure Sentry in your Next.js application, capture your first errors, logs and traces and view them in Sentry" | |
| description: "Learn how to set up and configure Sentry in your Next.js application, capture your first errors, logs and traces and view them in Sentry." |
logaretm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff here, I think this is great and really easy to follow without too much info overloading.
Just a quick note on tunnel configuration because I think it can be very confusing at its current state in the SDK and we could try to make it clear till we figure a way to solve it.
| // Use a fixed route (recommended) | ||
| tunnelRoute: "/monitoring", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
medium: saw that we recommend fixed tunnel routes here but then recommend the true option in the webpack setup, is this intended?
I think we can stick to the fixed recommendation in all pages and mention the random tunnel generation in case the user wants to avoid adblockers, sadly the random tunnel option means the user has no way of excluding it in the middleware as we don't wrap it in turbopack builds and may stop doing it for webpack next major as well.
Lots of info 😅 here, I think we can just recommend the fixed one for now and mention the random tunnel option as a note or a comment.
cc @chargome
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, i updated to default to /monitoring and added an expandable on the webpack page to talk about dynamic tunneling, great catch!
…ide-nextjs' of github.com:getsentry/sentry-docs into sergiydybskiy/devex-149-net-new-full-getting-started-guide-nextjs
…ide-nextjs' of github.com:getsentry/sentry-docs into sergiydybskiy/devex-149-net-new-full-getting-started-guide-nextjs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Step toggle aria-label has off-by-one error
The aria-label for step toggle buttons uses stepNumber after it has been incremented on line 101, causing a mismatch between the displayed step number and the accessibility label. For numbered steps, a step with data-step="1" will have aria-label="Toggle completion for step 2". Additionally, for unnumbered steps (where noNumber is true), the aria-label still references a step number, which is semantically incorrect for steps without a number.
src/components/stepConnector/index.tsx#L108-L109
sentry-docs/src/components/stepConnector/index.tsx
Lines 108 to 109 in 678f185
| btn.className = styles.stepToggle; | |
| btn.setAttribute('aria-label', `Toggle completion for step ${stepNumber}`); |
DESCRIBE YOUR PR
IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes:
LEGAL BOILERPLATE
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
EXTRA RESOURCES