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

Use canonicalUrl filter in Atom feed #272

Merged
merged 1 commit into from
Apr 22, 2024
Merged

Conversation

frankieroberto
Copy link
Contributor

Switching to using the custom canonicalUrl filter defined in this plugin rather than the absoluteUrl filter defined in the eleventy-plugin-rss plugin, so the URLs will include any pathPrefix defined in the Eleventy config.

This is because, I think, the two filters work in subtly different ways:

absoluteUrl resolves a path with a base url, which means that:

'/path/test' | absoluteUrl('https://example.test/prefix') returns https://example.test/path/test - as the initial / in the path indicates a root-relative url.

canonicalUrl appends the paths together, which means that:

'/path/test' | canonicalUrl('https://example.test/prefix/') returns https://example.test/prefix/path/test

An alternative solution would be to use the url filter from Eleventy or the HTML base plugin included with Eleventy 2.0+ - however that one still wouldn't work on the Atom feed as it only transforms links in HTML automatically, and so we’d have to use the htmlBaseUrl filter instead. 🤯

Switching to using the custom `canonicalUrl` filter defined in this plugin rather than the `absoluteUrl` filter defined in the `eleventy-plugin-rss` plugin, so the URLs will include any `pathPrefix` defined in the Eleventy config.
@frankieroberto
Copy link
Contributor Author

This was all pretty confusing. Perhaps long-term we should aim to remove the canonicalUrl custom filter and use one of the filters built in to Eleventy - but those are currently a bit confused, with many ways to do the same thing.

I think my ideal would be for item.url to return the full URL in all contexts (including any pathPrefix), with no need for any filters. Then if you didn't want a full URL, item.path could be exposed instead?

@paulrobertlloyd
Copy link
Collaborator

Before merging, do you want to try using the url filter in the Atom feed? It should work… though given the continued presence of canonicalUrl, maybe it won’t. (I agree, it’s all a bit of a mess; another area of 11ty which is a mess of unexpected behaviour).

@paulrobertlloyd
Copy link
Collaborator

paulrobertlloyd commented Apr 20, 2024

Oh wait! I think I know why you can’t! url almost gives you the destined outcome, but doesn’t return a full URL:

{{ "/path/to/file" | url }}
{# returns "/pathPrefix/path/to/file" #}

However, Atom feeds and other places require a fully resolved URL. That’s why I added canonicalUrl:

{{ "/path/to/file" | canonicalUrl }}
{# returns "https://website.example/pathPrefix/path/to/file" #}

Might want to double-check that is true, but from memory that’s what’s going on here.

@frankieroberto
Copy link
Contributor Author

@paulrobertlloyd yes, it's all very confusing!

As I understand it, url filter correctly adds the pathPrefix to item.url but doesn't include the full domain name. For that, you then need to pass it to another filter.

So, I think there’s (at least) 3 different ways to do this (tested all these locally):

  1. {{ item.url | url | absoluteUrl(options.url) }} - this uses the absoluteUrl filter from the RSS plugin and is what the Eleventy docs for the RSS plugin suggests (but that doesn't support the pathPrefix.

  2. ``{{ item.url | htmlBaseUrl(options.url) }} - this uses the [htmlBaseUrl`](https://www.11ty.dev/docs/plugins/html-base/) filter.

  3. {{ item.url | canonicalUrl }} - uses our custom canonicalUrl filter.

The other thing I’ve realised is that canonicalUrl ignores the pathPrefix setting altogether, but still works if you add the path prefix to the url setting of this Eleventy plugin. So you end up needing to set it in two places:

eleventyConfig.addPlugin(govukEleventyPlugin, {
  url: 'https://www.example.com/my-prefix/'
})

return {
  pathPrefix: '/my-prefix'
}

I think for consistency we should stick with canonicalUrl for now, but perhaps reconsider in future if any of this gets simpler within Eleventy itself?

🤯

@paulrobertlloyd
Copy link
Collaborator

paulrobertlloyd commented Apr 21, 2024

Yeah, let’s merge for now and review the use of this filter when Eleventy 3.0.0 comes out; I suspect there might be a lot of things we’ll do a bit differently 🤞

@frankieroberto frankieroberto merged commit 6e48d64 into main Apr 22, 2024
2 checks passed
@frankieroberto frankieroberto deleted the use-canonical-url branch April 22, 2024 08:58
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.

2 participants