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

Update sitemap to use relative links instead of localhost when missing site URL #1479

Merged
merged 7 commits into from
Nov 27, 2023

Conversation

caendesilva
Copy link
Member

Abstract

This PR makes so that when a site URL is null, or for localhost, we use relative links instead of absolute links using the fallback URL of http://localhost.

This change was especially complicated as there are many conflicting factors to consider here. Most noted in #1469.

I eventually landed on this as I think it's the least bad decision. Relative URLs are technically not allowed by the sitemap spec, but considering that the alternative (and current implementation) is to use localhost which will never resolve, I think it's more reasonable to use relative links that will hopefully resolve.

While localhost URLs are valid when building locally, this is one of the drawbacks of this route, as Hyde does not know if a build is for testing or production as that would add a lot of complexity without much value (an early design decision that it makes little sense to have different static site versions for local/prod)

Impact

Due to this change, when a site URL is not set to a custom value, instead of the following XML which is syntactically valid according to the spec, but which will never resolve when deployed:

<?xml version="1.0" encoding="UTF-8"?>
<urlset
	xmlns="https://www.sitemaps.org/schemas/sitemap/0.9" generator="HydePHP 1.3.4" processing_time_ms="0.33092498779297">
	<url>
		<loc>http://localhost/404.html</loc>
		<lastmod>2023-11-27T15:18:49+00:00</lastmod>
		<changefreq>daily</changefreq>
		<priority>0.5</priority>
	</url>
	<url>
		<loc>http://localhost/index.html</loc>
		<lastmod>2023-11-27T15:18:49+00:00</lastmod>
		<changefreq>daily</changefreq>
		<priority>1</priority>
	</url>
</urlset>

Instead, the output will now be as follows, which is technically not adhering to the spec, but crawlers should still be able to figure it out.

<?xml version="1.0" encoding="UTF-8"?>
<urlset
	xmlns="https://www.sitemaps.org/schemas/sitemap/0.9" generator="HydePHP 1.3.4" processing_time_ms="0.3211498260498">
	<url>
		<loc>404.html</loc>
		<lastmod>2023-11-27T15:18:49+00:00</lastmod>
		<changefreq>daily</changefreq>
		<priority>0.5</priority>
	</url>
	<url>
		<loc>index.html</loc>
		<lastmod>2023-11-27T15:18:49+00:00</lastmod>
		<changefreq>daily</changefreq>
		<priority>1</priority>
	</url>
</urlset>

Considerations

I, for now, decided against adding a build warning, for now, as that would likely spam the console and not be relevant and could overwhelm the console for new users making their first build.

Since the docs currently just specify that a site URL is needed to be specified to use sitemaps, they don't need to be updated.

Copy link

codecov bot commented Nov 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (79907bd) 100.00% compared to head (c2c3fd5) 100.00%.
Report is 4 commits behind head on master.

❗ Current head c2c3fd5 differs from pull request most recent head b79629e. Consider uploading reports for the commit b79629e to get more accurate results

Additional details and impacted files
@@              Coverage Diff              @@
##              master     #1479     +/-   ##
=============================================
  Coverage     100.00%   100.00%             
- Complexity      1730      3466   +1736     
=============================================
  Files            180       360    +180     
  Lines           4695      9400   +4705     
=============================================
+ Hits            4695      9400   +4705     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@caendesilva caendesilva marked this pull request as ready for review November 27, 2023 16:28
@caendesilva caendesilva force-pushed the add-sitemap-fallback-for-missing-site-url branch from 59ecfd0 to c2c3fd5 Compare November 27, 2023 16:30
@caendesilva caendesilva changed the title Add sitemap fallback to use relative links when missing a site url Update sitemap to use relative links instead of localhost when a site URL is not set Nov 27, 2023
@caendesilva caendesilva changed the title Update sitemap to use relative links instead of localhost when a site URL is not set Update sitemap to use relative links instead of localhost when no site URL is set Nov 27, 2023
@caendesilva caendesilva changed the title Update sitemap to use relative links instead of localhost when no site URL is set Update sitemap to use relative links instead of localhost when missing site URL Nov 27, 2023
@caendesilva caendesilva force-pushed the add-sitemap-fallback-for-missing-site-url branch from c2c3fd5 to b79629e Compare November 27, 2023 16:35
@caendesilva caendesilva merged commit 82265f5 into master Nov 27, 2023
18 checks passed
@caendesilva caendesilva deleted the add-sitemap-fallback-for-missing-site-url branch November 27, 2023 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sitemap links should probably fall back to relative links instead of localhost when no site URL is set
1 participant