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

Rework Next.js quick start guide (wizard) #12291

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

inventarSarah
Copy link
Collaborator

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.

  • Urgent deadline (GA date, etc.):
  • Other deadline:
  • None: Not urgent, can wait up to 1 week+

SLA

  • Teamwork makes the dream work, so please add a reviewer to your PRs.
  • Please give the docs team up to 1 week to review your PR unless you've added an urgent due date to it.
    Thanks in advance for your help!

PRE-MERGE CHECKLIST

Make sure you've checked the following before merging your changes:

  • Checked Vercel preview for correctness, including links
  • PR was reviewed and approved by any necessary SMEs (subject matter experts)
  • PR was reviewed and approved by a member of the Sentry docs team

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

Copy link

vercel bot commented Jan 9, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sentry-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 16, 2025 2:21pm
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
changelog ⬜️ Ignored (Inspect) Visit Preview Jan 16, 2025 2:21pm
develop-docs ⬜️ Ignored (Inspect) Visit Preview Jan 16, 2025 2:21pm

Copy link

codecov bot commented Jan 9, 2025

Bundle Report

Changes will increase total bundle size by 228 bytes (0.0%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
sentry-docs-server-cjs 10.37MB 234 bytes (0.0%) ⬆️
sentry-docs-client-array-push 9.3MB 6 bytes (-0.0%) ⬇️

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.

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.

docs/platforms/javascript/common/index.mdx Show resolved Hide resolved
@@ -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)
Copy link
Member

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.

Copy link
Collaborator Author

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)

Copy link
Member

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."

Copy link
Collaborator Author

@inventarSarah inventarSarah Jan 14, 2025

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).

Copy link
Member

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!

@@ -1,46 +1,30 @@
Add a button to a frontend component that throws an error:
## Step 3: Verify Your Setup
Copy link
Member

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.

Copy link
Collaborator Author

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

Copy link
Member

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.

Copy link
Collaborator Author

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)
Copy link
Member

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

Copy link
Collaborator Author

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

Copy link
Member

@chargome chargome left a comment

Choose a reason for hiding this comment

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

Nice rework!

Let's make sure to merge in #12293 before this one and also see if @lforst has any comments here

@inventarSarah
Copy link
Collaborator Author

@lfrost @chargome
For testing purposes, I have updated this quick start guide with some ideas from the team.

This also addresses some of your feedback, so I hope you can find some improvements in these changes too!

What's changed:

  • Step 1 should be Install (the old Step 1 wasn't actionable, which does not make sense for something called "Step" :D)
  • Test: moved the features list into an Expandable in the Install section
  • Test: moved the list in View Captured Data in Sentry into an Expandable
  • Test: moved the Expandable about Turbopack into the Verify section

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.
I need to revisit the page with refreshed eyes later again :)
In the meantime, please have a look if you want, and I will also ask Salma to do the same

Thank you!!

</Note>
</Alert>

You can always add or remove features manually later if needed. Though, the earlier you set these up, the better.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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?

Copy link
Collaborator Author

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?

Copy link
Member

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]>
@whitep4nth3r
Copy link
Contributor

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.

@inventarSarah
Copy link
Collaborator Author

inventarSarah commented Jan 16, 2025

@whitep4nth3r

Hello! This is sooo much better!

Yes, 💯

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, 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/

@codyde
Copy link

codyde commented Jan 16, 2025

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.

@lforst
Copy link
Member

lforst commented Jan 17, 2025

@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?"

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.

6 participants