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

feat(rss): add option to remove the trailing slash #6453

Merged
merged 3 commits into from
Mar 9, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/popular-rules-divide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
'@astrojs/rss': minor
---

Added `trailingSlash` option, to control whether the emitted URLs should have the trailing slash.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a precedent for verb tense on these? I haven't been picky, but I've now seen the equivalent of:

  • Added
  • Add
  • Adds
    in recent PRs, and it would be nice if we had a standard! Anyone have any thoughts on this?

(Would also make "slashes" plural to agree with plural URLs)

Suggested change
Added `trailingSlash` option, to control whether the emitted URLs should have the trailing slash.
Added `trailingSlash` option to control whether or not the emitted URLs should have trailing slashes.

AND in line 16 below: change to "won't have trailing slashes"



```js
import rss from '@astrojs/rss';

export const get = () => rss({
trailingSlash: false
});
```

By passing `false`, the emitted links won't have the trailing slash.
17 changes: 17 additions & 0 deletions packages/astro-rss/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ export function get(context) {
customData: '<language>en-us</language>',
// (optional) add arbitrary metadata to opening <rss> tag
xmlns: { h: 'http://www.w3.org/TR/html4/' },
// (optional) add trailing slashes to URLs (default: true)
trailingSlash: false
});
}
```
Expand Down Expand Up @@ -233,6 +235,21 @@ export async function get(context) {
}
```

### `trailingSlash`
ematipico marked this conversation as resolved.
Show resolved Hide resolved
ematipico marked this conversation as resolved.
Show resolved Hide resolved

Type: `boolean (optional)`
Default: "always"
ematipico marked this conversation as resolved.
Show resolved Hide resolved
bluwy marked this conversation as resolved.
Show resolved Hide resolved

By default, the library will add trailing slashes to the emitted URLs. To prevent this behavior, add `trailingSlash: false` to the `rss` function.

```js
import rss from '@astrojs/rss';

export const get = () => rss({
trailingSlash: false
});
```

---

For more on building with Astro, [visit the Astro docs][astro-rss].
Expand Down
7 changes: 5 additions & 2 deletions packages/astro-rss/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export type RSSOptions = {
customData?: z.infer<typeof rssOptionsValidator>['customData'];
/** Whether to include drafts or not */
drafts?: z.infer<typeof rssOptionsValidator>['drafts'];
trailingSlash?: z.infer<typeof rssOptionsValidator>['trailingSlash'];
};

type RSSFeedItem = {
Expand All @@ -54,6 +55,7 @@ type GlobResult = z.infer<typeof globResultValidator>;

const rssFeedItemValidator = rssSchema.extend({ link: z.string(), content: z.string().optional() });
const globResultValidator = z.record(z.function().returns(z.promise(z.any())));

const rssOptionsValidator = z.object({
title: z.string(),
description: z.string(),
Expand All @@ -77,6 +79,7 @@ const rssOptionsValidator = z.object({
drafts: z.boolean().default(false),
stylesheet: z.union([z.string(), z.boolean()]).optional(),
customData: z.string().optional(),
trailingSlash: z.boolean().default(true),
});

export default async function getRSS(rssOptions: RSSOptions) {
Expand Down Expand Up @@ -171,7 +174,7 @@ async function generateRSS(rssOptions: ValidatedRSSOptions): Promise<string> {
root.rss.channel = {
title: rssOptions.title,
description: rssOptions.description,
link: createCanonicalURL(site).href,
link: createCanonicalURL(site, rssOptions.trailingSlash, undefined).href,
};
if (typeof rssOptions.customData === 'string')
Object.assign(
Expand All @@ -183,7 +186,7 @@ async function generateRSS(rssOptions: ValidatedRSSOptions): Promise<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, site).href;
: createCanonicalURL(result.link, rssOptions.trailingSlash, site).href;
const item: any = {
title: result.title,
link: itemLink,
Expand Down
16 changes: 14 additions & 2 deletions packages/astro-rss/src/util.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,22 @@
import { z } from 'astro/zod';
import { RSSOptions } from './index';

/** Normalize URL to its canonical form */
export function createCanonicalURL(url: string, base?: string): URL {
export function createCanonicalURL(
url: string,
trailingSlash?: RSSOptions['trailingSlash'],
base?: string
): URL {
let pathname = url.replace(/\/index.html$/, ''); // index.html is not canonical
pathname = pathname.replace(/\/1\/?$/, ''); // neither is a trailing /1/ (impl. detail of collections)
if (!getUrlExtension(url)) pathname = pathname.replace(/(\/+)?$/, '/'); // add trailing slash if there’s no extension
if (trailingSlash === false) {
// remove the trailing slash
pathname = pathname.replace(/(\/+)?$/, '');
} else if (!getUrlExtension(url)) {
// add trailing slash if there’s no extension or `trailingSlash` is true
pathname = pathname.replace(/(\/+)?$/, '/');
}

pathname = pathname.replace(/\/+/g, '/'); // remove duplicate slashes (URL() won’t)
return new URL(pathname, base);
}
Expand Down
15 changes: 14 additions & 1 deletion packages/astro-rss/test/rss.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ describe('rss', () => {
const { body } = await rss({
title,
description,
drafts: true,
items: [phpFeedItem, { ...web1FeedItem, draft: true }],
site,
drafts: true,
Expand All @@ -116,6 +115,20 @@ describe('rss', () => {
chai.expect(body).xml.to.equal(validXmlResult);
});

it('should not append trailing slash to URLs with the given option', async () => {
const { body } = await rss({
title,
description,
items: [phpFeedItem, { ...web1FeedItem, draft: true }],
site,
drafts: true,
trailingSlash: false,
});

chai.expect(body).xml.to.contain('https://example.com/<');
chai.expect(body).xml.to.contain('https://example.com/php<');
});

it('Deprecated import.meta.glob mapping still works', async () => {
const globResult = {
'./posts/php.md': () =>
Expand Down