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

feat: Update setup docs for JS SDKs #6836

Merged
merged 9 commits into from
May 8, 2023
Merged

Conversation

mydea
Copy link
Member

@mydea mydea commented May 4, 2023

I took a bit of time to go through the browser JS docs pages, and just cleaned them up a bit/aligned them a bit in minor ways that I noticed.

Major things I did:

  • There were some places where we have not been using <PlatformIdentifier>, I updated those (that I found)
  • Some small spacing etc. adjustments
  • Removed/adjusted some version markers. IMHO we do not need to call out APIs that have been added in v7 or so anymore specifically (and we have plenty of things that are not marked as such but have been added only in v7).
  • Updated content for browser support to reflect reality better
  • Updated content for CDN bundle modifiers a bit

Closes #5351

@mydea mydea requested review from a team May 4, 2023 12:37
@mydea mydea self-assigned this May 4, 2023
@mydea mydea requested review from Lms24 and AbhiPrasad and removed request for a team May 4, 2023 12:37
@vercel
Copy link

vercel bot commented May 4, 2023

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 May 8, 2023 9:08am

@AbhiPrasad
Copy link
Member

Seems like this will close #5351?

Copy link
Contributor

@shanamatthews shanamatthews left a comment

Choose a reason for hiding this comment

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

Thanks so much for these cleanups @mydea !

This looks really good. Just left some small suggestions for cleanup and questions.

FYI, we are starting to add linting on the docs code snippets, so whitespace fixes should start happening automatically soon.

src/platforms/javascript/common/install/loader.mdx Outdated Show resolved Hide resolved
src/platforms/javascript/common/install/loader.mdx Outdated Show resolved Hide resolved
src/platforms/common/usage/sdk-fingerprinting.mdx Outdated Show resolved Hide resolved
src/platforms/common/usage/sdk-fingerprinting.mdx Outdated Show resolved Hide resolved
@mydea mydea force-pushed the fn/browser-js-improvements branch from c7d4cab to 0dcf4b6 Compare May 5, 2023 06:51
@mydea mydea changed the title feat: Minor cleanup of browser JS docs feat: Update setup docs for JS SDKs May 5, 2023
@@ -1,13 +1,16 @@
To use this SDK, initialize Sentry in your Remix entry points for both the client and server.

```typescript {filename: entry.client.tsx}
Create two files in the root directory of your project, `entry.client.tsx` and `entry.server.tsx` (if they don't exist yet). In these files, add your initialization code for the client-side SDK and server-side SDK, respectively.
Copy link
Member Author

Choose a reason for hiding this comment

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

@AbhiPrasad could you sanity check this?

@@ -26,17 +26,25 @@ If you're updating your Sentry SDK to the latest version, check out our [migrati

Create three files in the root directory of your project, `sentry.client.config.js`, `sentry.server.config.js` and `sentry.edge.config.js`. In these files, add your initialization code for the client-side SDK and server-side SDK, respectively. We've included some examples below.

For each configuration:
The three configuration types are mostly the same, except that some configuration, like the one for Session Replay, only works in `sentry.client.config.js`.
Copy link
Member Author

Choose a reason for hiding this comment

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

@lforst could you sanity check this?


```javascript {filename:sentry.(client|server|edge).js}
```javascript {tabTitle:Client} {filename:sentry.client.config.(js|ts)}
Copy link
Member

Choose a reason for hiding this comment

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

TIL

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 have one slight worry but let's see what happens first: Adding replay has a giant bundle size impact but we don't tell people here. They might be shocked when looking at their graphs at first. In the Next.js wizard I added a comment in the code saying like "if you don't plan on using the replay feature you can remove the integration".

Just a thought - we don't have to do this now but we could if we get a lot of issues complaining about bundle size again.

src/platforms/javascript/guides/nextjs/manual-setup.mdx Outdated Show resolved Hide resolved
mydea added 4 commits May 8, 2023 10:28
style(lint): Auto commit lint changes
style(lint): Auto commit lint changes
style(lint): Auto commit lint changes
style(lint): Auto commit lint changes
style(lint): Auto commit lint changes
style(lint): Auto commit lint changes
style(lint): Auto commit lint changes
style(lint): Auto commit lint changes
style(lint): Auto commit lint changes
style(lint): Auto commit lint changes
style(lint): Auto commit lint changes
style(lint): Auto commit lint changes
@mydea mydea force-pushed the fn/browser-js-improvements branch from 8ef6226 to b1ad4db Compare May 8, 2023 08:28
@mydea mydea force-pushed the fn/browser-js-improvements branch from 21bf873 to c4f1c83 Compare May 8, 2023 08:45
@mydea mydea merged commit 9d4c3c7 into master May 8, 2023
@mydea mydea deleted the fn/browser-js-improvements branch May 8, 2023 09:09
Lms24 pushed a commit that referenced this pull request May 8, 2023
---------

Co-authored-by: Shana Matthews <shana.l.matthews@gmail.com>
@github-actions github-actions bot locked and limited conversation to collaborators May 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[browser docs] Update "supported browsers" page
5 participants