fix(commerce): stop sending structured data to bots when explicitly disabled #1505
Conversation
📝 WalkthroughWalkthroughAdds a structured data control mechanism and utility to determine JSON-LD inclusion by mode ("disable for users" | "disable for all" | "always include"), and integrates it into PDP and PLP SEO components, deprecating the boolean Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
Tagging OptionsShould a new tag be published when this PR is merged?
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @commerce/sections/Seo/SeoPDPV2.tsx:
- Line 55: Remove the default assignment for structuredDataControl in the
destructuring of the component props so it can be undefined and allow the
existing fallback that checks ignoreStructuredData to operate; specifically, in
SeoPDPV2.tsx remove the `= "always include"` default on structuredDataControl
(so the nullish coalescing later that uses ignoreStructuredData will work), and
apply the same change to the other occurrences noted around the 78–84 region to
restore backward compatibility with ignoreStructuredData.
🧹 Nitpick comments (1)
commerce/utils/structuredData.ts (1)
7-16: Logic is correct but could be slightly clearer.The function correctly implements the intended behavior:
"always include"→ includes for all"disable for users"→ includes only for bots"disable for all"→ never includesThe compound boolean expression works but may be harder to parse at a glance.
🔎 Optional: More explicit structure for readability
export function shouldIncludeStructuredData( jsonLD: unknown, control: StructuredDataControl = "always include", isBot: boolean, ): boolean { if (!jsonLD) return false; - return control !== "disable for all" && - (control !== "disable for users" || isBot); + switch (control) { + case "disable for all": + return false; + case "disable for users": + return isBot; + case "always include": + default: + return true; + } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
commerce/sections/Seo/SeoPDPV2.tsxcommerce/sections/Seo/SeoPLPV2.tsxcommerce/utils/structuredData.ts
🧰 Additional context used
🧬 Code graph analysis (2)
commerce/sections/Seo/SeoPLPV2.tsx (1)
commerce/utils/structuredData.ts (2)
StructuredDataControl(2-5)shouldIncludeStructuredData(7-16)
commerce/sections/Seo/SeoPDPV2.tsx (1)
commerce/utils/structuredData.ts (2)
StructuredDataControl(2-5)shouldIncludeStructuredData(7-16)
🔇 Additional comments (6)
commerce/utils/structuredData.ts (1)
2-5: Clear and well-designed type definition.The three-mode approach provides better control than the previous boolean, aligning well with the PR objective to prevent duplicate structured data when custom LD+JSON is used.
commerce/sections/Seo/SeoPLPV2.tsx (3)
9-12: LGTM!Clean import of the new utility function and type from the shared module.
21-31: Good deprecation strategy and documentation.The deprecation notice on
ignoreStructuredDatawith clear guidance to usestructuredDataControlinstead is well done. The JSDoc for the new prop clearly explains the three modes and warns about third-party integration implications.
96-103: Backward compatibility handled correctly.The fallback logic properly maps:
structuredDataControltakes precedence when set- Legacy
ignoreStructuredData: true→"disable for users"(preserving original bot-only behavior)- Legacy
ignoreStructuredData: false/undefined→"always include"This ensures existing configurations continue to work as expected.
commerce/sections/Seo/SeoPDPV2.tsx (2)
9-12: LGTM!Consistent import pattern with SeoPLPV2.tsx.
27-38: Deprecation and new prop documented consistently with PLP.Good alignment between PDP and PLP interfaces.
- Replace boolean `ignoreStructuredData` with explicit `structuredDataControl` options - Fix bug where bots still received JSON-LD even when `ignoreStructuredData=true` - Add "always include" (default), "disable for users", and "disable for all" modes - Deprecate `ignoreStructuredData` with backward compatibility - Extract shared logic to `shouldIncludeStructuredData` utility
marcoferreiradev
force-pushed
the
feat/ignore-jsonld
branch
from
07b52df to
a77b9ad
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
commerce/sections/Seo/SeoPDPV2.tsx (1)
30-38: Deprecation and new API properly documented!The changes mirror the PLP implementation with clear deprecation notices and comprehensive JSDoc for the new property.
Note: The API structure differs slightly from SeoPLPV2 (which nests
structuredDataControlinsideConfigJsonLD), but this appears intentional given PDP's simpler configuration needs.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
commerce/sections/Seo/SeoPDPV2.tsxcommerce/sections/Seo/SeoPLPV2.tsxcommerce/utils/structuredData.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- commerce/utils/structuredData.ts
🧰 Additional context used
🧬 Code graph analysis (2)
commerce/sections/Seo/SeoPLPV2.tsx (1)
commerce/utils/structuredData.ts (2)
StructuredDataControl(2-5)shouldIncludeStructuredData(7-16)
commerce/sections/Seo/SeoPDPV2.tsx (1)
commerce/utils/structuredData.ts (2)
StructuredDataControl(2-5)shouldIncludeStructuredData(7-16)
🔇 Additional comments (7)
commerce/sections/Seo/SeoPLPV2.tsx (4)
9-12: LGTM!The imports correctly bring in the utility function and type definition needed for the new structured data control mechanism.
23-31: Well-documented deprecation and new API!The deprecation notice clearly directs users to the new
structuredDataControlproperty, and the JSDoc for the new property is comprehensive, explaining all three modes and potential integration impacts.
96-99: Backward compatibility correctly preserved!The mode derivation logic properly handles the deprecated
ignoreStructuredDataflag while respecting the newstructuredDataControlwhen provided. The mapping ofignoreStructuredData=trueto"disable for users"maintains the original behavior.
101-103: Clean integration of the utility function!The structured data inclusion logic correctly delegates to
shouldIncludeStructuredData, passing all required parameters and producing the expectedjsonLDsarray.commerce/sections/Seo/SeoPDPV2.tsx (3)
9-12: LGTM!The imports are correct and consistent with the companion PLP implementation.
78-80: Backward compatibility correctly implemented!The mode derivation properly handles the deprecated
ignoreStructuredDataflag with a clear comment. The logic correctly defaults to"always include"when neither prop is provided, maintaining expected behavior.
82-84: Clean utility integration!The structured data inclusion logic correctly uses
shouldIncludeStructuredDatawith proper parameter passing, consistent with the PLP implementation.
1 participant
What is this Contribution About?
Fixes a bug where
ignoreStructuredData=truewas still sending JSON-LD to search engine bots, causing duplicate structured data on sites with custom LD+JSON implementations.Changes:
ignoreStructuredDatawith explicitstructuredDataControlprop offering three modes:"always include"(default),"disable for users"(bots only),"disable for all"(completely disabled)shouldIncludeStructuredDatautilityBefore: Bots received duplicate
@type":"ProductDetailsPage"when custom structured data existedAfter: Sites can properly exclude section's structured data while maintaining custom implementations
Issue Link
N/A - Bug identified during bot testing with curl
Demonstration Link
Tested with Googlebot user-agent simulation - structured data now correctly excluded when configured
Summary by CodeRabbit
New Features
Deprecations
✏️ Tip: You can customize this high-level summary in your review settings.