Conversation
Inject TechArticle and BreadcrumbList schemas on all docs pages. Add FAQPage schema on the FAQ page for rich search results.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f1bf1a971
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .split("-") | ||
| .map((w: string) => w.charAt(0).toUpperCase() + w.slice(1)) | ||
| .join(" "); | ||
| breadcrumbItems.push({ name, url: `${baseUrl}/docs${currentPath}` }); |
There was a problem hiding this comment.
The breadcrumb schema currently creates an item URL for every slug segment via "/docs${currentPath}", but this repo has only a top-level docs index (docs/src/content/docs/docs/index.mdx) and no intermediate index pages like /docs/getting-started or /docs/features/capturing; those generated breadcrumb links therefore point to 404s on many pages. Because BreadcrumbList is meant to describe real navigable hierarchy, these dead links can cause search engines to ignore or distrust the structured data.
Useful? React with 👍 / 👎.
AvdLee
force-pushed
the
docs/seo-structured-data
branch
from
c328289 to
6f1bf1a
Compare
Skip intermediate URL segments that don't have index pages. Breadcrumbs now link: Home > Docs > Current Page.
There was a problem hiding this comment.
Pull request overview
Adds JSON-LD structured data to documentation pages to improve SEO and eligibility for rich results.
Changes:
- Injects
TechArticleJSON-LD on (non-home, non-404) docs pages. - Injects
BreadcrumbListJSON-LD for docs pages. - Injects
FAQPageJSON-LD on the FAQ page.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const breadcrumbItems = [ | ||
| { name: "Home", url: baseUrl }, | ||
| { name: "Docs", url: `${baseUrl}/docs` }, | ||
| ]; | ||
| if (title !== "RocketSim Docs" && title !== "404") { | ||
| breadcrumbItems.push({ name: title, url: pageUrl }); | ||
| } |
There was a problem hiding this comment.
The PR description says the BreadcrumbList is derived from the URL path hierarchy, but the implementation always emits only Home → Docs → <current title> and ignores intermediate segments (e.g. /docs/features/networking/... will still be a direct child of Docs). Either update the PR description or build breadcrumbs from the actual path/navigation hierarchy so the JSON-LD matches the intended behavior.
| if (breadcrumbItems.length > 1) { | ||
| schemas.push({ | ||
| "@context": "https://schema.org", | ||
| "@type": "BreadcrumbList", | ||
| itemListElement: breadcrumbItems.map((item, index) => ({ | ||
| "@type": "ListItem", | ||
| position: index + 1, | ||
| name: item.name, | ||
| item: item.url, | ||
| })), | ||
| }); | ||
| } |
There was a problem hiding this comment.
BreadcrumbList schema is still emitted on the docs 404 page because breadcrumbItems always contains at least Home and Docs, so breadcrumbItems.length > 1 is always true. Error pages generally shouldn’t publish structured data; consider skipping BreadcrumbList when title === "404" (or based on slug) alongside the TechArticle guard.
| if (isFaqPage && faqEntries.length > 0) { | ||
| schemas.push({ | ||
| "@context": "https://schema.org", | ||
| "@type": "FAQPage", | ||
| mainEntity: faqEntries.map((faq) => ({ |
There was a problem hiding this comment.
The FAQ JSON-LD is populated from a hard-coded list that does not match the actual FAQ page content (src/content/docs/docs/support/faq.md). This can lead to structured data being flagged as misleading by search engines. Consider sourcing the FAQ entries from the same content (e.g., frontmatter data, a shared JSON/data module imported by both the page and Head, or generating from the markdown headings/content) so the JSON-LD stays in sync.
| if (isFaqPage && faqEntries.length > 0) { | |
| schemas.push({ | |
| "@context": "https://schema.org", | |
| "@type": "FAQPage", | |
| mainEntity: faqEntries.map((faq) => ({ | |
| const faqData = Array.isArray(entry?.data?.faq) ? entry.data.faq : []; | |
| if (isFaqPage && faqData.length > 0) { | |
| schemas.push({ | |
| "@context": "https://schema.org", | |
| "@type": "FAQPage", | |
| mainEntity: faqData.map((faq) => ({ |
| { | ||
| schemas.map((schema) => ( | ||
| <script | ||
| type="application/ld+json" | ||
| is:inline | ||
| set:html={JSON.stringify(schema)} | ||
| /> | ||
| )) | ||
| } |
There was a problem hiding this comment.
set:html={JSON.stringify(schema)} injects raw JSON into a <script> tag. If any field (e.g. title/description) ever contains </script> or certain < sequences, it can break out of the script context and become an XSS vector. Consider escaping < (commonly replacing it with \u003c in the JSON string) before injecting, or using a safer serialization helper.
| const baseUrl = "https://www.rocketsim.app"; | ||
| const { entry } = Astro.props; | ||
| const title = entry?.data?.title ?? "RocketSim Docs"; | ||
| const description = entry?.data?.description ?? ""; | ||
| const slug = entry?.slug ?? ""; | ||
| const pageUrl = `${baseUrl}/${slug}`; |
There was a problem hiding this comment.
baseUrl is hard-coded here even though the project already centralizes the canonical site URL in src/config/config.json (and astro.config.ts sets site from it). This creates a second source of truth and makes structured data wrong in preview/staging environments. Prefer using Astro.site (configured via site) or importing @/config/config.json and using config.site.base_url for URL construction.
Skip BreadcrumbList structured data on 404 pages since error pages should not publish structured data. Escape '<' as \u003c in JSON-LD to prevent potential XSS if page titles ever contain </script>.
ngnijland
left a comment
ngnijland
left a comment
There was a problem hiding this comment.
We should re-use the existing StructuredData component and type the schema's properly (not unknown). The faq comment is an optional suggestion, since I'm not 100% if it is possible in a nice way/
There was a problem hiding this comment.
Issue:
We already have an StructuredData component where the schema objects are typed using TypeScript. You can find it here. We should add new schema types there and use that component for consistency.
| const faqEntries = isFaqPage | ||
| ? [ | ||
| { | ||
| q: "Does RocketSim offer out-of-the App Store distribution?", | ||
| a: "Yes. Get in touch via support@rocketsim.app.", | ||
| }, | ||
| { | ||
| q: "Are there non-recurring subscriptions?", | ||
| a: "Yes, you can consider buying Team Licenses at rocketsim.app/team-insights.", | ||
| }, | ||
| { | ||
| q: "Can I reimburse my App Store subscription?", | ||
| a: "Yes, RocketSim offers Team Licenses as an alternative at rocketsim.app/team-insights.", | ||
| }, | ||
| { | ||
| q: "Do you provide commercial or team licenses?", | ||
| a: "Yes, check out Team Licenses at rocketsim.app/team-insights.", | ||
| }, | ||
| { | ||
| q: "Can I buy a lifetime RocketSim license?", | ||
| a: "No, but you can earn a lifetime license through SwiftLee Weekly's referral program. Join the newsletter and follow the instructions.", | ||
| }, | ||
| { | ||
| q: "Why is my video not accepted by App Store Connect?", | ||
| a: "Videos are optimized for App Previews following Apple's specifications. Make sure to use a Simulator matching a device from the App Preview specifications.", | ||
| }, | ||
| { | ||
| q: "Why wouldn't I just use xcrun simctl?", | ||
| a: "RocketSim uses xcrun simctl under the hood but enhances the output with touches, device bezels, and a visual interface for quicker access.", | ||
| }, | ||
| { | ||
| q: "Why does RocketSim need screen recording permissions?", | ||
| a: "RocketSim is sandboxed and needs screen recording permissions to read Simulator window titles, which it uses to determine the currently active Simulator.", | ||
| }, | ||
| { | ||
| q: "Where can I report bugs or feature requests?", | ||
| a: "Issues and feature requests are managed on GitHub at github.com/AvdLee/RocketSimApp/issues.", | ||
| }, | ||
| { | ||
| q: "How can I get PNG images instead of JPEG?", | ||
| a: "Disable App Store Connect (ASC) Optimization. ASC requires JPEG images without alpha layer.", | ||
| }, | ||
| { | ||
| q: "Why are my iPad captures upside-down?", | ||
| a: "RocketSim cannot detect landscape-left or landscape-right and defaults to one rotation. Rotate your Simulator twice and restart the recording.", | ||
| }, | ||
| { | ||
| q: "Can I create transparent captures?", | ||
| a: "Yes, disable App Preview Optimized and set your background color to transparent.", | ||
| }, | ||
| { | ||
| q: "Network Speed Control isn't working, what can I do?", | ||
| a: "Quit Xcode, all Simulators, and RocketSim, then reopen them. Check System Settings → Privacy & Security for pending permissions. On macOS Sequoia, enable the Network Extension in System Settings → General → Login Items & Extensions.", | ||
| }, | ||
| ] | ||
| : []; |
There was a problem hiding this comment.
Suggestion:
Having these questions and answers here as a duplicate to the faq in the markdown files is bound to diverge in the future. Maybe we can come up with a way to have the questions and answers declared in one place and then automatically placed in both schema values and the markdown file. I'm not sure how, we could do some research there.
Move TechArticle, BreadcrumbList, and FAQPage structured data from inline logic in Head.astro into the shared StructuredData component with proper TypeScript types. Extract FAQ entries into a shared JSON data file to avoid duplication with the markdown FAQ page. Apply XSS escaping to all JSON-LD output globally. Addresses reviewer feedback on PR #934.
Resolve conflict in Head.astro by keeping both the structured data delegation from our branch and the OG image meta tags from master.
Summary
Changes
docs/src/components/starlight/Head.astroto generate and inject structured dataTest plan
npm run buildpasses