Allows sitemap output on root of public folder

[pedrolamas]

Description

All this PR does is allow developers to set the output option on the gatsby-sitemap-plugin to "" (empty string) so that the files can be written to the root of the public folder.

Some (like me) might prefer that instead of creating a subfolder to store the files, plus this mitigates a separate issue where the index file always points to the root folder instead of the specified output folder.

Documentation

https://www.gatsbyjs.com/plugins/gatsby-plugin-sitemap/

Related Issues

Fixes #31095

[pedrolamas]

FYI, I've tested this locally and works fine with an empty string but will fail with null value, hence why I didn't put that as allowed!

[KyleAMathews]

Curious why an empty string vs /?

[pedrolamas]

@KyleAMathews I tested with "/" and noticed that the header that was created had double slashes in it (so it showed "//sitemap-index.xml"), using a blank string fixed that!

[KyleAMathews]

Ah ok, that's the bug that should be fixed then here —

gatsby/packages/gatsby-plugin-sitemap/src/gatsby-ssr.js

Line 20 in a2a9dc9

href={withPrefix(withoutTrailingSlash(output) + `/sitemap-index.xml`)}

When joining paths, it's best to use path.join as that then takes care automatically of any possible doubled slashes e.g:

➜  gatsby-plugin-sitemap git: ✗ node
Welcome to Node.js v16.0.0.
Type ".help" for more information.
> const path = require(`path`)
undefined
> path.join(`/`, `/`)
'/'

[pedrolamas]

I do wonder if this issue is showing in other bits, given this:

gatsby/packages/gatsby-plugin-sitemap/src/gatsby-ssr.js

Lines 5 to 6 in a2a9dc9

	// TODO: Remove for v3 - Fix janky path/asset prefixing
	const withPrefix = withAssetPrefix || fallbackWithPrefix

[moonmeister]

I was going to summit the pr to convert to path.join but it probably is a better solution than an empty string. You're welcome to do it here.

[pedrolamas]

AFAIK, path.join() should not be used for this, as we are talking about urls and not actual paths, and in Windows land, that can causes even bigger problems:

Having said that, path.posix.join() works, but once again, that is NOT the correct way to do this!

[pedrolamas]

This looks more like it!

gatsby/deprecated-packages/gatsby-plugin-guess-js/src/gatsby-ssr.js

Lines 7 to 12 in 54d4721

	function urlJoin(...parts) {
	return parts.reduce((r, next) => {
	const segment = next == null ? `` : String(next).replace(/^\/+/, ``)
	return segment ? `${r.replace(/\/$/, ``)}/${segment}` : r
	}, ``)
	}

But I do wonder if there's already some dependency that Gatsby is using to do this correctly?

[moonmeister]

Good call, stark overflow looks like it has some good suggestions. https://stackoverflow.com/questions/16301503/can-i-use-requirepath-join-to-safely-concatenate-urls

[pedrolamas]

Given the comments from @KyleAMathews, I've changed the code to fix when output: "/" to ensure that the link is correctly created.

I've also added a small test to ensure this keeps the expected behavior.

[moonmeister]

Thanks for the fix and especially for the tests.

[moonmeister]

I haven't run the code but looks good.

[KyleAMathews]

Ah yeah, posix.join is what we want cause windows. We use that elsewhere in core https://github.com/gatsbyjs/gatsby/search?q=posix.join

Nice work @pedrolamas !

[KyleAMathews]

Published in gatsby-plugin-sitemap@4.1.0-next.1 (so you can install it with gatsby-plugin-sitemap@next)