-
Notifications
You must be signed in to change notification settings - Fork 281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve SEO lib with seo
option in loader data response
#591
Conversation
This comment has been minimized.
This comment has been minimized.
brand: product?.vendor, | ||
name: product?.title, | ||
}, | ||
} satisfies SeoConfig<Product>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
satisfies FTW 👁️👁️
packages/hydrogen/src/seo/seo.ts
Outdated
@@ -45,7 +45,18 @@ export function Seo({debug}: SeoProps) { | |||
const routeInfo = {...routeMatch, ...location}; | |||
|
|||
if (handle === undefined || handle.seo === undefined) { | |||
return []; | |||
if ( | |||
routeMatch.data === undefined || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: if (!route?.data?.seo)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
@@ -0,0 +1,153 @@ | |||
import {createElement} from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to ts
WHY are these changes introduced?
Fixes #577
This PR implements a new API for SEO as described in the above issue.
I've also added tests for this component which required
react-testing
setup in vitest, though I've kept this setup as minimal as possible to start.WHAT is this pull request doing?
HOW to test your changes?
Post-merge steps
Checklist