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

@astrojs/rss uses trailing slash on home page when it shouldn't #10994

Closed
1 task
patrickreiner opened this issue May 9, 2024 · 8 comments · Fixed by #11050
Closed
1 task

@astrojs/rss uses trailing slash on home page when it shouldn't #10994

patrickreiner opened this issue May 9, 2024 · 8 comments · Fixed by #11050
Assignees
Labels
- P2: nice to have Not breaking anything but nice to have (priority) pkg: rss Related to the `@astrojs/rss` package (scope)

Comments

@patrickreiner
Copy link

Astro Info

Astro                    v4.8.2
Node                     v20.12.1
System                   macOS (arm64)
Package Manager          npm
Output                   hybrid
Adapter                  @astrojs/vercel/serverless
Integrations             @astrojs/sitemap

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

In my astro.config.mjs I set:

export default defineConfig({
...
trailingSlash: 'never',
})

and in my vercel.json I set:

{
"cleanUrls": true,
...
"trailingSlash": false
}

and in my rss.xml.js I set:

export async function GET(context) {
return rss({
trailingSlash: false,
})
}

and yet, the RSS feed still puts a slash on the home page, viewable here:

http://astro-rss-trailing-slash.patrickreiner.com/rss.xml

What's the expected result?

RSS home page should not have a trailing slash.

Should be:

https://astro-rss-trailing-slash.patrickreiner.com

Not:

https://astro-rss-trailing-slash.patrickreiner.com/

Link to Minimal Reproducible Example

https://github.com/patrickreiner/astro-rss-trailing-slash

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label May 9, 2024
@mingjunlu mingjunlu added pkg: rss Related to the `@astrojs/rss` package (scope) - P3: minor bug An edge case that only affects very specific usage (priority) and removed needs triage Issue needs to be triaged labels May 10, 2024
@mingjunlu mingjunlu self-assigned this May 10, 2024
@BryceRussell
Copy link
Member

Hello! 👋

There was a discussion about this on another issue here: #10723 (comment)

In summary: The urls are treated as the same, there is no difference between https://example.com and https://example.com/ except visual

@mingjunlu
Copy link
Contributor

@BryceRussell Thanks! I somehow missed that issue

@BryceRussell
Copy link
Member

@mingjunlu No worries!

@patrickreiner I am going to go ahead and close this since this is expected behavior. If you have any other issues feel free to open another issue 😀

@BryceRussell BryceRussell added - P2: nice to have Not breaking anything but nice to have (priority) and removed - P3: minor bug An edge case that only affects very specific usage (priority) labels May 10, 2024
@mingjunlu mingjunlu assigned mingjunlu and unassigned mingjunlu May 11, 2024
@patrickreiner
Copy link
Author

Hi @BryceRussell, thank you, I agree with your label of "nice to have" where it's not breaking anything, but it would be nice to have the visual consistency. @astrojs/sitemap for example used to work like this as well (where the home page always had a trailing backslash) but that behavior was recently corrected as you can see here https://astro-rss-trailing-slash.patrickreiner.com/sitemap-0.xml

@BryceRussell
Copy link
Member

You're right! Sorry, I did not realize the sitemap had a similar change recently and did not consider that this could be changed even if it is only visual. For reference, there is a workflow for triaging issues that I try to follow, but I am still learning 😅 I will re-open this issue

@BryceRussell BryceRussell reopened this May 11, 2024
@github-actions github-actions bot added the needs triage Issue needs to be triaged label May 11, 2024
@BryceRussell BryceRussell removed the needs triage Issue needs to be triaged label May 11, 2024
@BryceRussell
Copy link
Member

It looks like this issue happens because the links are transformed from a string, to a URL, and then back to a string

return new URL(pathname, base);

if (typeof result.link === 'string') {
// If the item's link is already a valid URL, don't mess with it.
const itemLink = isValidURL(result.link)
? result.link
: createCanonicalURL(result.link, rssOptions.trailingSlash, site).href;
item.link = itemLink;

Example:

// Returns 'http://example.com/'
new URL('http://example.com').toString()

@V3RON
Copy link
Contributor

V3RON commented May 13, 2024

@BryceRussell That's exactly the reason. I'm willing to work on it if you don't mind 😉

@Fryuni
Copy link
Member

Fryuni commented May 14, 2024

For awareness, the recent change on the sitemap was not just cosmetic like it is here; in that case, it affects SEO by having the canonical URL of a page mismatched between the page and the sitemap.

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) pkg: rss Related to the `@astrojs/rss` package (scope)
Projects
None yet
5 participants