-
Notifications
You must be signed in to change notification settings - Fork 290
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
Juanpprieto/stable sitemap #2589
Conversation
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
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.
I didn't review the code, but 👍 for
- Removing the example
- Moving to
'2024-10'
rather thanunstable
- Updating the skeleton
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.
Looking at the deployments, the XML produced looks legit. However, I tried to follow a link from the metaObject
page and got a 404 https://01j9cc8nc12y73gvtmjaar74p0-bdc62392032d96aafe24.myshopify.dev/custom_pages/mono-skis, is that expected?
@@ -9,6 +9,8 @@ const SITEMAP_PREFIX = `<?xml version="1.0" encoding="UTF-8"?> | |||
<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9" xmlns:xhtml="http://www.w3.org/1999/xhtml">`; | |||
const SITEMAP_SUFFIX = `</urlset>`; | |||
|
|||
const SITEMAP_STOREFRONT_API_VERSION = '2024-10'; |
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.
Ideally this version should not be hardcoded and it should either:
Accepts and sfapiVersion
or consume any version passed to our clients constructions via server.ts
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.
Since this is going out with the 2024-10 release, as I understand it we will no longer need to provide the storefrontApiVersion
override to the storefront.query
call, so I'll remove this const
and rebase this against #2570
746aaae
to
215fd7d
Compare
Update
Stabilize
getSitemap
,getSitemapIndex
and implement on skeletonUpgrade instructions
getSitemapIndex
at/app/routes/[sitemap.xml].tsx
getSitemap
at/app/routes/sitemap.$type.$page[.xml].tsx
For a reference implementation please see the skeleton template sitemap routes:
PR description
WHY are these changes introduced?
Closes https://github.com/orgs/Shopify/projects/5093/views/16?pane=issue&itemId=79743679 and https://github.com/orgs/Shopify/projects/5093/views/16?pane=issue&itemId=79604624
WHAT is this pull request doing?
getSitemap
andgetSitemapIndex
utilitiesHOW to test your changes?
Visit the
/sitemap.xml
route and nested sitemap urlsChecklist