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 608 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: sentry-docs-server-cjsAssets Changed:
Files in
App Routes Affected:
view changes for bundle: sentry-docs-client-array-pushAssets Changed:
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
a-hariti
left a comment
a-hariti
left a comment
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
@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? |
…l hierarchy, remove repetition
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Deployment failed with the following error: |
|
Deployment failed with the following error: |
…tsentry/sentry-docs into platform-redirect-privacy-fix
| if (isValidPlatform) { | ||
| return redirect(`/platforms/${requestedPlatform}${pathname}`); | ||
| if (validPlatform) { | ||
| return redirect(`${validPlatform.url}${pathname}`); |
There was a problem hiding this comment.
Double slashes in redirect URLs due to concatenation
High Severity
The URL construction creates double slashes. validPlatform.url ends with a trailing slash (e.g., /platforms/javascript/ from nodeToPlatform which uses '/' + n.path + '/'), and pathname starts with a leading slash (from sanitizeNext which ensures paths start with /). Concatenating them produces URLs like /platforms/javascript//session-replay/privacy/. The old code avoided this by using /platforms/${key}${pathname} where key had no trailing slash.
Additional Locations (1)
| filteredGuides = supportedGuides.filter( | ||
| guide => !notSupported.includes(`${platformEntry.key}.${guide.name}`) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Missing supported whitelist filtering for guides
Medium Severity
The filtering logic only checks notSupported frontmatter but ignores the supported frontmatter field. The isSupported function in frontmatter.ts treats supported as a whitelist - when defined, only explicitly listed platforms/guides are supported. Pages like apple/enriching-events/screenshots have supported: ["apple.ios", "apple.visionos"], meaning only those guides should appear. The new code would show any guide with a valid path that's not in notSupported, ignoring the whitelist and potentially displaying guides that explicitly should be hidden.
|
Deployment failed with the following error: |
2 participants
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: