Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
IEvangelist
force-pushed
the
dapine/issue-681-jsonld-structured-data
branch
from
9eca9f2 to
8ba551d
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds build-time Schema.org JSON-LD generation for Aspire’s Starlight-driven site to improve structured metadata discoverability.
Changes:
- Introduces a shared
getStructuredData()utility that builds JSON-LD for different page types (home, docs articles, FAQ, release notes, Aspire Conf). - Injects the generated JSON-LD via the shared Starlight
Headcomponent.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/frontend/src/utils/structured-data.ts | New JSON-LD builder/serializer plus FAQ extraction and release-version parsing logic. |
| src/frontend/src/components/starlight/Head.astro | Emits the JSON-LD <script type="application/ld+json"> when route data is available. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I'm testing this locally and I'm not seeing the sameAs links show up on the rendered homepage. We only should need it on the home page and the community page not needed on every article. It basically describes "us" - the org and our social channels belonging together. |
Addresses PR feedback: - Beth: sameAs should only appear on home + community pages. Added an explicit community schema (Organization + WebPage) so it shows there too, and strengthened home detection. - Copilot: added Vitest coverage for getStructuredData() (home, community, generic TechArticle, release-version mapping, FAQ extraction, and JSON escaping of <, >, &, U+2028/U+2029).
|
LGTM |
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
Nice work — well-structured utility with good XSS escaping and test coverage. Three suggestions inline (two nits + one CI gap).
| .replace(/^[-*]\s+/, ''); | ||
|
|
There was a problem hiding this comment.
If a FAQ answer contains H3+ sub-headings (e.g. ### Details), the ### marker will survive into the plain-text answer since only H2 is used to split questions. Consider stripping heading markers from answer lines:\n\nsuggestion\n const cleanedLine = cleanInlineMarkdown(trimmed)\n .replace(/^#{3,6}\\s+/, '')\n .replace(/^\\d+\\.\\s+/, '')\n .replace(/^[-*]\\s+/, '');\n
| publisher: buildPublisher(siteUrl), | ||
| url: pageUrl, |
There was a problem hiding this comment.
Nit: buildPublisher(siteUrl) is called twice here, creating two identical objects. Could assign to a local variable and reuse it for both author and publisher.\n\nsuggestion\n const publisher = buildPublisher(siteUrl);\n\n return {\n '@type': 'TechArticle',\n headline: route.entry.data.title,\n description,\n author: publisher,\n publisher,\n url: pageUrl,\n inLanguage: language,\n about: buildSoftwareApplication(siteUrl),\n };\n
There was a problem hiding this comment.
The new structured-data.vitest.test.ts file isn't wired into test:unit in package.json, so it won't run in CI or via pnpm test. Suggest adding:\n\njson\n\"test:unit\": \"pnpm test:unit:contracts && pnpm test:unit:components && pnpm test:unit:docs && pnpm test:unit:api-markdown && pnpm test:unit:structured-data\",\n\"test:unit:structured-data\": \"vitest run --config vitest.config.ts tests/unit/structured-data.vitest.test.ts\",\n
Summary
Closes #681
Notes