EPMRPP-113100 || Update the Schema.org markup for the documentation#1103
Open
maria-hambardzumian wants to merge 3 commits into
Open
Conversation
maria-hambardzumian
requested review from
AmsterGet and
pressayuliya
as code owners
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 53 minutes and 15 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughA new Docusaurus plugin framework adds Schema.org JSON-LD generation to documentation pages by scanning markdown files, extracting structured metadata, building JSON-LD objects, injecting them into page headers, and validating the output during CI with a dedicated validation script. Configuration and dependencies updated to support the feature. Changes
Sequence DiagramsequenceDiagram
participant Build as Docusaurus Build
participant Plugin as Schema Plugin
participant Parser as Parser Module
participant Layout as DocItem Layout
participant Output as HTML Output
Build->>Plugin: Load plugin
Plugin->>Plugin: Scan markdown/mdx files
Plugin->>Plugin: Extract frontmatter & paths
Plugin->>Parser: Resolve schema types (mapping.js)
Parser-->>Plugin: Type mappings
Plugin->>Parser: Parse FAQPage/HowTo/SoftwareApp (parser.js)
Parser-->>Plugin: Extracted structures (faq, howto, etc.)
Plugin->>Build: Store schema entries in global data
Build->>Layout: Render doc page
Layout->>Layout: Read doc metadata & frontmatter
Layout->>Layout: Load schema entry from plugin data
Layout->>Layout: buildJsonLd() generates JSON-LD array
Layout->>Output: Inject <script type="application/ld+json">
Layout-->>Output: Render DocItem with JSON-LD
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
scripts/validate-schema.js (2)
65-82: Reduce duplicated errors when amainEntityentry is not a valid object.If a
Questionentry isnullor not an object,checkFaqPagestill runs all three sub-checks and emits three errors for the same malformed item. Short-circuiting once shape is invalid keeps the report signal-to-noise higher.♻️ Proposed refactor
questions.forEach((question, index) => { - if (!question || question['@type'] !== 'Question') { + if (!question || typeof question !== 'object') { + errors.report(file, `FAQPage mainEntity[${index}] is not an object`); + return; + } + if (question['@type'] !== 'Question') { errors.report(file, `FAQPage mainEntity[${index}] `@type` != Question`); } - if (!question || !question.name) { + if (!question.name) { errors.report(file, `FAQPage mainEntity[${index}] missing name`); } - const answer = question && question.acceptedAnswer; + const answer = question.acceptedAnswer; if (!answer || answer['@type'] !== 'Answer' || !answer.text) { errors.report(file, `FAQPage mainEntity[${index}].acceptedAnswer invalid`); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/validate-schema.js` around lines 65 - 82, The checkFaqPage function currently emits multiple errors for the same malformed mainEntity entry; modify the loop in checkFaqPage so that after converting node.mainEntity to questions you first validate whether question is a non-null object and has '@type' === 'Question' (using the existing question and questions identifiers and errors.report) and if that validation fails emit a single descriptive error (e.g., about invalid or non-object mainEntity[${index}]) then continue to the next iteration; only run the subsequent name and acceptedAnswer checks (using question.name and the answer variable/acceptedAnswer/@type/text) when that initial shape check passes to avoid duplicate reports.
153-159: Type stats under-report@graph-wrapped payloads.The
countsByTypekey is derived from the outer node's@type, but when the JSON-LD block is a@graphcontainer the wrapper itself has no@type, so every such page is bucketed under(no@type)even though its children carry real types (e.g.,Article,BreadcrumbList). Counting the children (which are already validated on line 155) would make the summary accurate.♻️ Proposed refactor
toArray(parsed).forEach((node) => { const graphChildren = node && node['@graph'] ? toArray(node['@graph']) : [node]; - graphChildren.forEach((child) => validateNode(child, errors, file)); - - const typeKey = toArray(node && node['@type']).join(',') || '(no `@type`)'; - stats.countsByType[typeKey] = (stats.countsByType[typeKey] || 0) + 1; + graphChildren.forEach((child) => { + validateNode(child, errors, file); + const typeKey = toArray(child && child['@type']).join(',') || '(no `@type`)'; + stats.countsByType[typeKey] = (stats.countsByType[typeKey] || 0) + 1; + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/validate-schema.js` around lines 153 - 159, The current stats logic computes typeKey from the outer node (variable node/@type), which causes `@graph-wrapped` documents to be counted as "(no `@type`)"; change the logic in the block that builds graphChildren (where validateNode is called) to, when node['@graph'] exists, iterate graphChildren and derive each child's typeKey from toArray(child['@type']).join(',') || '(no `@type`)' and increment stats.countsByType for each child, otherwise keep the existing single-node counting; ensure you still call validateNode(child, errors, file) as before and update stats.countsByType in the same scope so graph-wrapped child types are correctly tallied.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 55-62: The package.json currently allows "cheerio": "^1.0.0" which
resolves to cheerio@1.2.0 requiring Node >=20.18.1 while your engines.node is
">=18.0"; update package.json to either set "engines.node" to ">=20.18.1" (to
match cheerio) or change the cheerio dependency to a version compatible with
Node 18 (e.g., pin "cheerio" to a specific older release) and run install to
regenerate lockfile; ensure you reference the "cheerio" entry in package.json
and the "engines.node" field when making the change.
In `@src/lib/schemaOrg.js`:
- Around line 198-206: buildJsonLd can emit a fallback schema for pages that
were intentionally skipped because it resolves types even when entry is missing;
change buildJsonLd to early-return an empty array when entry is falsy so no
schema is emitted for skipped docs. Specifically, in buildJsonLd, add a guard
(e.g., if (!entry) return []) before calling resolveTypes and using
BUILDERS/hasEnoughContent so that resolveTypes(entry, frontMatter) and
subsequent BUILDERS[type] mapping never run for absent/skipped entries.
In `@src/theme/DocItem/Layout/index.js`:
- Around line 27-34: The new Layout file uses JSX but is named index.js and also
triggers lint rules for array keys and prop spreading; rename
src/theme/DocItem/Layout/index.js to index.jsx (or adjust the project lint rule
to allow JSX in .js) and in the component ensure stable keys and avoid broad
prop spreading: keep the jsonLdBlocks map using a stable unique key (e.g., use
an explicit id field from each block instead of `index` in the Head key) and
replace the `...props` spread when rendering OriginalLayout with explicit prop
names passed through (or selectively spread only known safe props) to satisfy
the prop-spread lint rule; locate symbols jsonLdBlocks, Head, OriginalLayout,
and props in the file to apply these changes.
---
Nitpick comments:
In `@scripts/validate-schema.js`:
- Around line 65-82: The checkFaqPage function currently emits multiple errors
for the same malformed mainEntity entry; modify the loop in checkFaqPage so that
after converting node.mainEntity to questions you first validate whether
question is a non-null object and has '@type' === 'Question' (using the existing
question and questions identifiers and errors.report) and if that validation
fails emit a single descriptive error (e.g., about invalid or non-object
mainEntity[${index}]) then continue to the next iteration; only run the
subsequent name and acceptedAnswer checks (using question.name and the answer
variable/acceptedAnswer/@type/text) when that initial shape check passes to
avoid duplicate reports.
- Around line 153-159: The current stats logic computes typeKey from the outer
node (variable node/@type), which causes `@graph-wrapped` documents to be counted
as "(no `@type`)"; change the logic in the block that builds graphChildren (where
validateNode is called) to, when node['@graph'] exists, iterate graphChildren
and derive each child's typeKey from toArray(child['@type']).join(',') || '(no
`@type`)' and increment stats.countsByType for each child, otherwise keep the
existing single-node counting; ensure you still call validateNode(child, errors,
file) as before and update stats.countsByType in the same scope so graph-wrapped
child types are correctly tallied.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a74417d-d650-4700-918e-4b880aa4f8b4
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
.github/workflows/ci.ymldocusaurus.config.jspackage.jsonplugins/plugin-schema-org/index.jsplugins/plugin-schema-org/mapping.jsplugins/plugin-schema-org/parser.jsscripts/validate-schema.jssrc/lib/schemaOrg.jssrc/theme/DocItem/Layout/index.js
Comment on lines
+55
to
+62
| "cheerio": "^1.0.0", | ||
| "fast-glob": "^3.3.2", | ||
| "gray-matter": "^4.0.3", | ||
| "mdast-util-to-string": "^4.0.0", | ||
| "remark-mdx": "^3.0.0", | ||
| "remark-parse": "^11.0.0", | ||
| "unified": "^11.0.4", | ||
| "unist-util-visit": "^5.0.0" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Declared package Node engine:"
jq -r '.engines.node' package.json
echo
echo "Resolved newly added dependency versions from package-lock.json, if present:"
for dep in cheerio fast-glob gray-matter mdast-util-to-string remark-mdx remark-parse unified unist-util-visit; do
resolved=""
if [ -f package-lock.json ]; then
resolved="$(jq -r --arg dep "$dep" '.packages["node_modules/" + $dep].version // empty' package-lock.json)"
fi
if [ -n "$resolved" ]; then
echo "$dep@$resolved"
npm view "$dep@$resolved" engines --json || true
else
range="$(jq -r --arg dep "$dep" '.devDependencies[$dep] // empty' package.json)"
echo "$dep range: $range"
npm view "$dep@$range" version engines --json || true
fi
echo
doneRepository: reportportal/docs
Length of output: 405
Update engines.node to match resolved cheerio dependency or constrain the cheerio range.
The resolved version of cheerio@1.2.0 (from the caret range ^1.0.0) requires node >=20.18.1, but the project declares node >=18.0. Either raise engines.node to >=20.18.1 or pin cheerio to a version compatible with Node 18.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 55 - 62, The package.json currently allows
"cheerio": "^1.0.0" which resolves to cheerio@1.2.0 requiring Node >=20.18.1
while your engines.node is ">=18.0"; update package.json to either set
"engines.node" to ">=20.18.1" (to match cheerio) or change the cheerio
dependency to a version compatible with Node 18 (e.g., pin "cheerio" to a
specific older release) and run install to regenerate lockfile; ensure you
reference the "cheerio" entry in package.json and the "engines.node" field when
making the change.
Comment on lines
+198
to
+206
| export function buildJsonLd({ siteConfig, metadata, frontMatter, entry }) { | ||
| if (!metadata || !metadata.permalink) return []; | ||
|
|
||
| const context = { siteConfig, metadata, frontMatter, entry }; | ||
| const types = resolveTypes(entry, frontMatter).map((t) => String(t).trim()); | ||
|
|
||
| return types | ||
| .filter((type) => BUILDERS[type] && hasEnoughContent(type, entry)) | ||
| .map((type) => BUILDERS[type](context)); |
There was a problem hiding this comment.
Do not emit fallback schema for skipped docs.
The plugin intentionally skips draft/unlisted docs, but buildJsonLd still falls back to TechArticle when entry is missing. That can expose structured data for pages the plugin excluded and can hide lookup mismatches.
Proposed guard
export function buildJsonLd({ siteConfig, metadata, frontMatter, entry }) {
if (!metadata || !metadata.permalink) return [];
+ if (frontMatter && (frontMatter.draft === true || frontMatter.unlisted === true)) return [];
+ if (!entry) return [];
const context = { siteConfig, metadata, frontMatter, entry };
const types = resolveTypes(entry, frontMatter).map((t) => String(t).trim());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/schemaOrg.js` around lines 198 - 206, buildJsonLd can emit a fallback
schema for pages that were intentionally skipped because it resolves types even
when entry is missing; change buildJsonLd to early-return an empty array when
entry is falsy so no schema is emitted for skipped docs. Specifically, in
buildJsonLd, add a guard (e.g., if (!entry) return []) before calling
resolveTypes and using BUILDERS/hasEnoughContent so that resolveTypes(entry,
frontMatter) and subsequent BUILDERS[type] mapping never run for absent/skipped
entries.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
Chores