Skip to content
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: several sitemap related issues #441

Merged
merged 3 commits into from
May 10, 2021

Conversation

moonmeister
Copy link
Contributor

@moonmeister moonmeister commented May 2, 2021

  1. I have fixed a couple small bugs. As noted in Support for v4 of gatsby-plugin-sitemap #438 the default sitemap name has changed to (sitemap-index.xml) though I am trying to revert the default path change from /sitemap to / in fix(gatsby-plugin-sitemap): Sitemap path bug gatsbyjs/gatsby#31184. If this change probably requires a major version bump as it would be breaking.

  2. Because of the above change in gatsby-plugin-sitemap I tried to set the sitemap option on my site and it would not change from the default. Reading the code I realized there was a bug caused by the if statement I've fixed this so the custom sitemap path can be set without declaring the host as well.

  3. I removed the dependency on nodes old url package to make this more compatible with modern node as well.

  4. I added logic to allow the custom sitemap to be a relative path and prefix the host.

  5. I've put in logic to correctly handle Gatsby's pathPrefix feature.

If you'd like me to split any of this up into smaller PRS or remove anything let me know. Thanks.

Fixes #438

@mdreizin
Copy link
Owner

Hi folks! Thank you very much for contribution. It looks good to me. I am going to merge it. I am really sorry for the delayed review and response.

Copy link
Owner

@mdreizin mdreizin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@mdreizin mdreizin merged commit e04abbc into mdreizin:master May 10, 2021
@mdreizin
Copy link
Owner

mdreizin commented May 10, 2021

Please wait for a release in a couple of minutes, github-actions[bot] will post a comment here.

@github-actions
Copy link

🎉 This PR is included in version 1.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@holm holm mentioned this pull request May 18, 2021
@wildpow wildpow mentioned this pull request May 27, 2021
@josephmarkus
Copy link

This change breaks when used with gatsby-plugin-sitemap 3.3.0 with the following:

> 80 |       mergedOptions.sitemap = new URL(mergedOptions.sitemap.startsWith(pathPrefix) ? mergedOptions.sitemap : path.posix.join(pathPrefix, mergedOptions.sitemap), mergedOptions.host).toString()

12:48:39 PM:
  79 |     } catch {

12:48:39 PM:
ERROR "gatsby-plugin-robots-txt" threw an error while running the onPostBuild lifecycle:

12:48:39 PM:
  78 |       new URL(mergedOptions.sitemap)

12:48:39 PM:
Cannot read property 'startsWith' of null

12:48:39 PM:
  83 |

12:48:39 PM:
     |                                                             ^

12:48:39 PM:
  82 |   }

12:48:39 PM:
  81 |     }

12:48:39 PM:
  TypeError: Cannot read property 'startsWith' of null

12:48:39 PM:
    [www]/[gatsby]/src/utils/api-runner-node.js:434:22

12:48:39 PM:
  - gatsby-node.js:80 Object.onPostBuild

Why is this breaking change not reflected in the version of this package update?

@moonmeister
Copy link
Contributor Author

moonmeister commented Jun 3, 2021

Please see: #451 or open a new issue if appropriate

@fmaiabatista
Copy link

Hi there, I have just started using the plugin and was confused by it's output differing from the docs I found in Gatsby's website. I expected them to live in example.com/sitemap.xml but they are instead located in example.com/sitemap/sitemap-index.xml.

Can you please update the docs/README according to the latest changes? I know this is recent but it helps whoever finds themselves in the same situation as me!

Thanks,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for v4 of gatsby-plugin-sitemap
4 participants