Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
7f00563 to
ca92a79
Compare
There was a problem hiding this comment.
Pull request overview
Adds initial documentation and UI support for the new Nitro JavaScript SDK within the docs site, including platform icon support and platform-specific source maps guidance.
Changes:
- Add Nitro platform icon mappings (
nitro+javascript-nitro) to the platform icon component. - Add a new Nitro guide page (
/platforms/javascript/guides/nitro/) and Nitro-specific source maps include content. - Bump
platformiconsdependency to a version which includes the Nitro icon assets.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/platformIcon.tsx | Adds Nitro SVG imports and maps javascript-nitro to the Nitro icon. |
| package.json | Updates platformicons to ^9.4.0 to pick up Nitro icon assets. |
| pnpm-lock.yaml | Lockfile updates reflecting the dependency bump (plus additional transitive changes). |
| platform-includes/sourcemaps/primer/javascript.nitro.mdx | Adds Nitro-specific source maps “primer” include content. |
| platform-includes/sourcemaps/overview/javascript.nitro.mdx | Adds Nitro-specific source maps “overview” include content (includes troubleshooting link). |
| docs/platforms/javascript/guides/nitro/index.mdx | Adds the main Nitro SDK setup/feature guide page. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
inventarSarah
left a comment
There was a problem hiding this comment.
Thanks, looks good overall!
I left a couple of suggestions for you to review.
Two more points:
- If it's not on your to-do list already anyway, please also have another look at the navigation and remove the pages we don't need for Nitro (for example, Session Replay).
- If you have the time, you could update the guide to use the new split layout. You can see how that works in almost any JavaScript quick start guide (example). Otherwise, I will update it once I revisit the page to clean up the "Features" section 👍
| `@sentry/nitro` automatically generates and uploads source maps when you build your Nitro application for production, so that errors in Sentry contain readable stack traces. | ||
|
|
||
| To configure source map uploads, provide your Sentry auth token, organization, and project slugs to the `withSentryConfig` wrapper: | ||
|
|
There was a problem hiding this comment.
I believe the code block is missing?
There was a problem hiding this comment.
So it's referring to the second code snippet in the tabbed code below, but I think this would be confusing.
I split the code snippets into two snippets, and move paragraphs around so it is less confusing.
There was a problem hiding this comment.
Thank you, now it's clearer 👍
|
@inventarSarah Noticed there was a lot of sidebar/page Is that the best way to do that? I also addressed the rest of your comments, thanks for the review! |
inventarSarah
left a comment
There was a problem hiding this comment.
Thanks for the updates! The changes look good to me (only left 1 comment) -- and no worries about pr size for these changes.
I noticed most of the other SDKs have main nav items "AI Agent Monitoring" and "Crons" which Nitro currently does not -- just double-checking if this looks correct to you.
| </SplitSectionCode> | ||
| </SplitSection> | ||
| </SplitLayout> | ||
|
|
There was a problem hiding this comment.
The Source Maps section is now missing.
You can add it as the last section under "2 Configuration" using an h3 for the title
| `@sentry/nitro` automatically generates and uploads source maps when you build your Nitro application for production, so that errors in Sentry contain readable stack traces. | ||
|
|
||
| To configure source map uploads, provide your Sentry auth token, organization, and project slugs to the `withSentryConfig` wrapper: | ||
|
|
There was a problem hiding this comment.
Thank you, now it's clearer 👍
Adds the Nitro SDK docs, it was released in
10.51.0IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.