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

RSS integration does not respect trailingSlash configuration option #6427

Closed
1 task
njakob opened this issue Mar 5, 2023 · 3 comments · Fixed by #6453
Closed
1 task

RSS integration does not respect trailingSlash configuration option #6427

njakob opened this issue Mar 5, 2023 · 3 comments · Fixed by #6453
Assignees
Labels
- P2: nice to have Not breaking anything but nice to have (priority)

Comments

@njakob
Copy link

njakob commented Mar 5, 2023

What version of astro are you using?

2.0.17

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Linux

Describe the Bug

While playing with the RSS integration (@astrojs/rss, version 2.1.1), I realized that it always forces trailing slashes even when the trailingSlash option is set to never.

This behavior only applies when the link isn't already a valid URL. However, considering common usages, including examples provided in the documentation, the current implementation forces trailing slashes.

const itemLink = isValidURL(result.link)
? result.link
: createCanonicalURL(result.link, site).href;

Looking at the documentation, the trailingSlash option should not alter trailing slashes in this case. However, this option is already used in the sitemap integration (@astrojs/sitemap) to alter the behavior of trailing slashes for production builds and is not limited to the dev server as suggested. This has been changed by #4553.

Therefore, for consistency, it would make sense to also apply the same logic to the URLs generated for RSS feeds.

You can observe this in the following minimal reproducible example where adding trailingSlash: 'never' in the configuration affects the sitemap generation but not the RSS feed.

<?xml version="1.0" encoding="UTF-8"?>
<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9" xmlns:news="http://www.google.com/schemas/sitemap-news/0.9" xmlns:xhtml="http://www.w3.org/1999/xhtml" xmlns:image="http://www.google.com/schemas/sitemap-image/1.1" xmlns:video="http://www.google.com/schemas/sitemap-video/1.1">
   <url><loc>https://example.com/</loc></url>
   <url><loc>https://example.com/blog/test</loc></url>
</urlset>
<rss version="2.0">
  <channel>
    <title>Buzz’s Blog</title>
    <description>A humble Astronaut’s guide to the stars</description>
    <link>https://example.com/</link>
    <item>
      <title>Test</title>
      <link>https://example.com/test/</link>
      <guid>https://example.com/test/</guid>
      <pubDate>Sun, 05 Mar 2023 17:53:53 GMT</pubDate>
    </item>
    <item>
      <title>My page of content</title>
      <link>https://example.com/blog/test/</link>
      <guid>https://example.com/blog/test/</guid>
      <pubDate>Sun, 05 Mar 2023 00:00:00 GMT</pubDate>
    </item>
  </channel>
</rss>

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-l6d4mc

Participation

  • I am willing to submit a pull request for this issue.
@ematipico ematipico added the - P2: nice to have Not breaking anything but nice to have (priority) label Mar 7, 2023
@ematipico ematipico self-assigned this Mar 7, 2023
@matthewp
Copy link
Contributor

matthewp commented Mar 7, 2023

The different between the sitemap integration and RSS is that @astrojs/rss is not an integration. So it doesn't have access to your Astro configuration at all. It only knows what you tell it.

So I guess it would make sense to have a trailingSlash option as part of the RSS API. As annoying as that would be to use.

@njakob
Copy link
Author

njakob commented Mar 7, 2023

That's an excellent point, I didn't consider that aspect @matthewp

Having this as an option would make sense. However, what would you then expose? Simply having trailingSlash as a boolean since all options (always, never, ignore) might not make sense in this case?

import rss, { pagesGlobToRssItems } from '@astrojs/rss';

export async function get(context) {
  return rss({
    trailingSlash: false,
    title: 'Buzz’s Blog',
    description: 'A humble Astronaut’s guide to the stars',
    site: context.site,
    items: await pagesGlobToRssItems(import.meta.glob('./blog/*.md'))
  });
}

Moreover, it is not possible to access the trailingSlash configuration from the context within the API route, correct?

@matthewp
Copy link
Contributor

matthewp commented Mar 7, 2023

boolean sgtm. That's right, you don't have access to the astro config from an API route, so that's why we can't do this automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: nice to have Not breaking anything but nice to have (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants