-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Rework Next.js quick start guide (wizard) #12291
base: master
Are you sure you want to change the base?
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
|
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.
@@ -32,11 +35,33 @@ Select which Sentry features you'd like to install in addition to Error Monitori | |||
|
|||
</PlatformSection> | |||
|
|||
<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!
|
||
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
</Note> | ||
</Alert> | ||
|
||
You can always add or remove features manually later if needed. Though, the earlier you set these up, the better. |
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.
You can always add or remove features manually later if needed. Though, the earlier you set these up, the better. | |
You can always add or remove features manually later if needed, but the setup is easiest if features are enabled from the start. |
Not 100% sure, but it seems a bit "unclear" why it is better (from a users point of view). Maybe this is a bit clearer?
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.
We could also remove the Alert ("This guide assumes that you enable all features ...") and instead write in basic text:
This guide assumes that you enable all features and allow the wizard to create an example page and route. You can add or remove features at any time, but setting them up now will save you the effort of configuring them manually later.
@mydea what do you think?
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.
sounds good to me!
Co-authored-by: Francesco Gringl-Novy <[email protected]>
Hello! This is sooo much better! There's one thing I noticed that could be clearer. The page title for the wizard setup is 'Next.js' and the page title for the manual setup is 'Manual setup'. For (probably SEO purposes) and also for clarity, could we have 'Next.js' in the title for manual setup as well? So maybe the quick start title could be 'Next.js: Quick Start' and the manual setup title could be 'Next.js: Manual Setup'. When I was on the manual setup page, I had to look to the sidebar or the breadcrumbs to make sure I was in the right place. |
Yes, 💯
Yes, we will definitely change the titles! For now, we need to keep them as they are because the title influences the name on the menu. For example, now the root page for next.js is our quick start guide called "Next.js". When we rename that, the new name will show up in the sidebar navigation but also in the overview lists, like on this page https://docs.sentry.io/ |
I love how this is coming together. Really happy with it. A suggestion to consider that i'd love @lforst perspective on - when I landed on this page; I wasnt entirely sure what was supported in Next.JS specifically (which is a super common question these days with things like RSC + Server Actions, as an example). We do a great job about calling out those configs in the Manual setup. I think theres a lot of value in a sentence on top that says something like "Sentry has out of the box support for Pages and App Router features. Features include React Server Components, Server Actions, Error Boundaries, Sourcemaps, and Tunneling" and link to the relevant content in each one of those. Just a suggestion; we can adjust the words. Much of that is handled in the wizard; but my thinking is if someone lands here - they'd get a clear answer. Feels like a positie the Next.JS experience. |
@codyde I think the reason why we never put such a section there is because we always supported everything (except for in transitionary periods). I never saw the value behind reciting what Next.js features there are. I question the value of explicitly stating what we support, when we support everything. I might be wrong though. I think the question that needs more answering is "What data should I see in Sentry (when everything is set up correctly)? And how do I use that data?" |
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