Fixing JS framework formatting for redirect pages #14914
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Bundle ReportChanges will increase total bundle size by 3.01kB (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: sentry-docs-client-array-pushAssets Changed:
view changes for bundle: sentry-docs-server-cjsAssets Changed:
Files in
App Routes Affected:
|
| const platformCopy = {...platform_, url: guide.url}; | ||
| // Update the platform reference to use the copy | ||
| Object.assign(platform_, platformCopy); | ||
| break; |
There was a problem hiding this comment.
Bug: Platform URL Mutation Causes Framework Inheritance Issues
The code mutates the original platform_ object's url property within the filter callback, despite the stated intent to avoid mutation. This modifies shared state from extractPlatforms(rootNode) and can lead to unexpected side effects. As a result, JavaScript frameworks derived from a mutated javascriptPlatform may inherit an incorrect, guide-specific URL instead of the main platform URL.
Additional Locations (1)
There was a problem hiding this comment.
Overall I love the end result 😍
We should just make the solution more generic and clean up the filter logic and unnecessary mutations (as per my other comments)
PS: I can implement my suggested changes if you want 🤙 @sfanahata
| let title = defaultTitle; | ||
|
|
||
| // get rid of irrelevant platforms for the `next` path | ||
| const platformList = extractPlatforms(rootNode).filter(platform_ => { |
There was a problem hiding this comment.
I think it will be a lot more freeing to switch the type of this list from "platforms" to "platforms or guides"
from there, I see two options:
- the nesting will either be rendered from a flat list similar to now,
preferably with some flag indicating that it should be nested,
which will also open the door for a more generic solution without the hardcoded JS logic
OR
- keep the the "platforms" type along with an inner filter over its guides, I platform would be retained in the filter if it has at least one relevant guide (the Dart/Flutter situation).
Then adjust the rendering accordingly
I tend to prefer approach 2, much cleaner
| javascriptPlatform.guides?.forEach(guide => { | ||
| const guideKey = `javascript.${guide.name}`; | ||
| if (!notSupported.includes(guideKey)) { | ||
| jsFrameworks.push({ |
There was a problem hiding this comment.
feels like a lot of repetition : )
@a-hariti - Yes please, if it doesn't feel like too much extra work, I'd love for you to take on the changes. Vibe coding obvs only gets me so far. 😊 |
|
@a-hariti - any updates on progress here? |
DESCRIBE YOUR PR
Summary
Enhanced the platform redirect system to show individual JavaScript frameworks (React, Vue, Angular, etc.) as separate, nested options under the main JavaScript platform. This provides users with a more customized and framework-specific experience when selecting their platform. Also added better logic to check for guide paths because Dart also wasn't showing up due to being nested.
Problem
When users clicked platform-redirect links (like /platform-redirect/?next=/session-replay/privacy/), they only saw top-level platforms (JavaScript, Android, React Native). This didn't reflect that many JavaScript frameworks have different support levels for specific features, and users couldn't easily select their specific framework for a more tailored experience. They also did not see Dart, which was nested under a Dart/Flutter guide.
Solution
Automatic Framework Detection: The system now reads the
notSupportedfrontmatter from JavaScript documentation pages to determine which frameworks support each feature. Added more robust logic to search for guides under each platform if main platform paths are empty.Visual Nesting: JavaScript frameworks are indented under the main JavaScript platform for clear hierarchy
Smart Positioning: JavaScript and its frameworks appear at the end of the platform list
Framework-Specific URLs: Each framework links to the main JavaScript documentation for that feature
Example pages:
https://sentry-docs-git-platform-redirect-privacy-fix.sentry.dev/platform-redirect/?next=/session-replay/configuration/#network-details
https://sentry-docs-git-platform-redirect-privacy-fix.sentry.dev/platform-redirect/?next=/session-replay/privacy/
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: