-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Rework Next.js quick start guide (wizard) #12291
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
Bundle ReportChanges will increase total bundle size by 228 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
lforst
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.
I love that we give this page some attention! Since this page is so critical though, we should be rather selective how much we bombard users with information. Additionally, the information we give them needs to be bulletproof. Anything in the wrong place will spark immense confusion.
|
|
||
| <PlatformSection supported={["javascript.nextjs"]}> | ||
|
|
||
| ## Step 1: Choose Your Sentry Features (Optional) |
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.
I would honestly remove this entire section. It's irrelevant to the setup in the docs since it's pretty much always gonna be the same because the wizard will already let you choose.
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.
Please see my response to a similar comment here: #12291 (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.
There are very conflicting opinions on adding a paragraph like this. IIRC we flipped back and fourth 4 times on having this paragraph vs not having it in the last 2 years.
I personally, don't think SDK onboarding is the place to go into that stuff. I do however see the value behind capturing people who know nothing about Sentry yet. How about instead of having a 3 point paragraph, we do a simple sentence like, "This guide will tell you how to hook up your application with the Sentry SDK. If you want to know more about Sentry itself go here."
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.
Yes, it's a hot topic, as I've learned :)! There are already discussions about moving these features to another location (e.g. an SDK landing page). And then, as you wrote, for example, link to it from the quick start page.
However, since we currently don't yet have a place for it, I'd like to leave the information here.
Or do you know of a page we could link to?
If not, is it okay for you to leave it like this for the time being?
I'm also working on the Next.js quick start guide for manual setup -> this page will also need to communicate the features to the user (including onboarding option checkboxes). How we will solve this in detail will probably also influence the quick start guide for the wizard setup (and vice versa).
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.
Makes sense, we can keep it for now!
platform-includes/getting-started-install/javascript.nextjs.mdx
Outdated
Show resolved
Hide resolved
platform-includes/getting-started-install/javascript.nextjs.mdx
Outdated
Show resolved
Hide resolved
platform-includes/getting-started-next-steps/javascript.nextjs.mdx
Outdated
Show resolved
Hide resolved
| @@ -1,46 +1,30 @@ | |||
| Add a button to a frontend component that throws an error: | |||
| ## Step 3: Verify Your Setup | |||
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.
I don't like this section because the wizard will tell you to do this. We should avoid putting information in multiple places. If the wizard is missing information that we would have added here, we should add it to the wizard instead. My working theory is that once people run the wizard command, they will not look back into the docs until they are done with looking at the error in Sentry. At which point all of this text is already irrelevant. What's more important in that moment is the Next Steps.
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.
I understand you're suggesting we simplify the guide to only include "Prerequisites," "Install," and "Next Steps." -- Did I get this right?
This is a new idea, and I'd like to share it with the others to get their input as well (hopefully tomorrow).
I see the advantages of having a shorter guide and relying more on the wizard.
However, there are a few points we need to be mindful about:
- once the terminal window in which the wizard ran is closed, all its information is gone; the quick start guide is always accessible
- "they will not look back into the docs until they are done with looking at the error in Sentry" - on the other hand, we probably also have users who don't read all the text in the terminal (I'm such a candidate, for example). Especially everything that comes after the green Success message may be perceived as "not important"; ideally, we can make both of these user types happy
- many users who use this type of guide are probably new and don't know much about Sentry yet. So the quick start guide is a great place to introduce them to features beyond error monitoring and provide links to more information -> but we have also talked about providing this info somewhere else (eg. on a landing page Have quick start as a menu item #12289)
this is also relevant to your comment on the features: /#discussion_r1910162943
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.
These points make sense. We can keep it, but I would advocate for half a sentence like "If you haven't done so through the setup wizard, go through the following steps to....". That way people will know not to do the same thing twice.
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.
Thank you -- I'll rewrite this section for sure because I now see that it can be misleading!
| Let's confirm that Sentry is working properly and sending data to your Sentry project by using the example page and route created by the installation wizard. | ||
|
|
||
| And throw an error in an API route: | ||
| 1. Open the example page at [http://localhost:3000/sentry-example-page](http://localhost:3000/sentry-example-page) |
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.
another problem: it may not always be localhost:3000 depending on the app
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.
I've updated this to be more like what we currently have in the wizard
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.
platform-includes/getting-started-install/javascript.nextjs.mdx
Outdated
Show resolved
Hide resolved
Co-authored-by: Charly Gomez <[email protected]>
|
@lfrost @chargome This also addresses some of your feedback, so I hope you can find some improvements in these changes too! What's changed:
I like that the guide is more actionable now and that we can still include information without scaring the user with an overly long page (by using the Expandables). However, I feel like there are too many boxes now and that it might not be pleasant to look at. Thank you!! |
platform-includes/getting-started-install/javascript.nextjs.mdx
Outdated
Show resolved
Hide resolved
platform-includes/getting-started-install/javascript.nextjs.mdx
Outdated
Show resolved
Hide resolved
Co-authored-by: Francesco Gringl-Novy <[email protected]>
whitep4nth3r
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 work. I love how the full guide fits completely on my weird vertical type monitor as a bonus. ✅
|
|
||
| After the wizard setup is completed, the SDK will automatically capture unhandled errors, and monitor performance. | ||
| You can also <PlatformLink to="/usage/">manually capture errors</PlatformLink>. | ||
| - [**Error Monitoring**](/product/issues) (always enabled): Automatically report errors, |
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.
I know Issues is our error monitoring product, but clicking on 'Error Monitoring' and being directed to the 'Issues' page might be confusing for those less familiar with Sentry.
What do you think about having this anchor text say 'issues' and providing some context in the description?
e.g. "Issues (always enabled): Sentry's core error monitoring product that automatically reports errors . . ."
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.
Let's do as you suggested! This is also a good idea because we also write about this "Issues" page under "Need help locating the captured errors in your Sentry project?"
Updated it 👍
| - [**Tracing**](/product/tracing): Track software performance while seeing the | ||
| impact of errors across multiple systems. For example, distributed tracing | ||
| allows you to follow a request from the frontend to the backend and back. | ||
| - [**Session Replay**](/product/explore/session-replay/web/getting-started/): |
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.
What do you think about linking to https://sentry-docs-git-smi-quick-startnextjs.sentry.dev/product/explore/session-replay/web/ instead of the /getting-started/ page here? I feel like it provides more context and feels more inline with the content on two links above.
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.
Yes, good idea!
Updated 👍
platform-includes/getting-started-install/javascript.nextjs.mdx
Outdated
Show resolved
Hide resolved
|
|
||
| </Alert> | ||
| - Create config files with the default `Sentry.init()` calls for all runtimes (Node.js, Browser, and Edge) | ||
| - Add a Next.js instrumentation hook to your project (`instrumentation.ts`) |
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.
| - Add a Next.js instrumentation hook to your project (`instrumentation.ts`) | |
| - Addition of Next.js instrumentation hook to your project (`instrumentation.ts`) |
platform-includes/getting-started-install/javascript.nextjs.mdx
Outdated
Show resolved
Hide resolved
platform-includes/getting-started-install/javascript.nextjs.mdx
Outdated
Show resolved
Hide resolved
platform-includes/getting-started-install/javascript.nextjs.mdx
Outdated
Show resolved
Hide resolved
platform-includes/getting-started-next-steps/javascript.nextjs.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.
Hey @inventarSarah, this looks great! I added some comments/suggestions, feel free to push back/reject any of them. 🏄♂️
codyde
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.
This looks great! I looked over the other feedback and saw you incorporated it all. This is all good on my side.
LGTM!
DESCRIBE YOUR PR
Reworked the Next.js Getting Started Guide into a Quick Start Guide.
See #12288 (and parent #11859)
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