-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
fix(gatsby-plugin-sitemap): include path prefix #12852
fix(gatsby-plugin-sitemap): include path prefix #12852
Conversation
Include path prefix in the index sitemap created by gatsby-plugin-sitemap.
@gatsbyjs/core is there any chance someone can take a look at this? |
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.
LGTM! thanks for fixing! let's wait for tests to finish
} | ||
const prefix = `/test` | ||
await onPostBuild({ graphql, pathPrefix: prefix }, options) | ||
expect(sitemapSpy.mock.calls[0][0].hostname).toEqual( |
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: don't need to change this (we do use it in different places as well), but for future, please use
expect(sitemapSpy).toBeCalledWith(
expect.objectContaining({
hostname: `${queryResult.data.site.siteMetadata.siteUrl}${prefix}`
})
)
reaching to .mock.calls[0][0]
works, but is less readable
Description
The Gatsby sitemap plugin can be configured to split up all of the sites URLs into smaller sitemaps, all being referenced from a main "index" sitemap. Unfortunately, this index sitemap doesn't take the
pathPrefix
option into account and will never link to the correct "child" sitemaps.As
sitemap.js
doesn't have an option forpathPrefix
, this change appends thepathPrefix
to the site'shostname
when more than one sitemap needs to be created.