From ed0b46f96faf144fe0946bce1528f4d605a4a42c Mon Sep 17 00:00:00 2001 From: Nate Moore Date: Fri, 21 Jan 2022 16:38:48 -0600 Subject: [PATCH] Fix pre-generated RSS URLs (#2443) * Allow pre-generated urls to be passed in rss feeds * Fix variable name * Add isValidURL helper function * Remove scary RegEx and tidy up code * add test for using pregenerated urls * fix: allow rss to be called multiple times * test: normalize rss feed in test * chore: add changeset Co-authored-by: Zade Viggers <74938858+zadeviggers@users.noreply.github.com> Co-authored-by: zadeviggers --- .changeset/flat-mayflies-ring.md | 5 +++ packages/astro/src/core/build/page-data.ts | 41 +++++++++++-------- packages/astro/src/core/ssr/rss.ts | 16 +++++--- packages/astro/src/core/util.ts | 9 ++++ packages/astro/test/astro-sitemap-rss.test.js | 9 +++- .../src/pages/episodes/[...page].astro | 20 +++++++++ 6 files changed, 75 insertions(+), 25 deletions(-) create mode 100644 .changeset/flat-mayflies-ring.md diff --git a/.changeset/flat-mayflies-ring.md b/.changeset/flat-mayflies-ring.md new file mode 100644 index 000000000..8cba31592 --- /dev/null +++ b/.changeset/flat-mayflies-ring.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fix bug with RSS feed generation. `rss()` can now be called multiple times and URLs can now be fully qualified. diff --git a/packages/astro/src/core/build/page-data.ts b/packages/astro/src/core/build/page-data.ts index 5b3605c24..8a40277f8 100644 --- a/packages/astro/src/core/build/page-data.ts +++ b/packages/astro/src/core/build/page-data.ts @@ -78,26 +78,31 @@ export async function collectPagesData(opts: CollectPagesDataOptions): Promise { +async function getStaticPathsForRoute(opts: CollectPagesDataOptions, route: RouteData): Promise<{ paths: string[]; rss?: RSSResult[] }> { const { astroConfig, logging, routeCache, viteServer } = opts; if (!viteServer) throw new Error(`vite.createServer() not called!`); const filePath = new URL(`./${route.component}`, astroConfig.projectRoot); diff --git a/packages/astro/src/core/ssr/rss.ts b/packages/astro/src/core/ssr/rss.ts index a61df7841..269ffbdf4 100644 --- a/packages/astro/src/core/ssr/rss.ts +++ b/packages/astro/src/core/ssr/rss.ts @@ -1,7 +1,7 @@ import type { RSSFunction, RSS, RSSResult, FeedResult, RouteData } from '../../@types/astro'; import { XMLValidator } from 'fast-xml-parser'; -import { canonicalURL, PRETTY_FEED_V3 } from '../util.js'; +import { canonicalURL, isValidURL, PRETTY_FEED_V3 } from '../util.js'; /** Validates getStaticPaths.rss */ export function validateRSS(args: GenerateRSSArgs): void { @@ -48,8 +48,10 @@ export function generateRSS(args: GenerateRSSArgs): string { if (!result.title) throw new Error(`[${srcFile}] rss.items required "title" property is missing. got: "${JSON.stringify(result)}"`); if (!result.link) throw new Error(`[${srcFile}] rss.items required "link" property is missing. got: "${JSON.stringify(result)}"`); xml += `<![CDATA[${result.title}]]>`; - xml += `${canonicalURL(result.link, site).href}`; - xml += `${canonicalURL(result.link, site).href}`; + // If the item's link is already a valid URL, don't mess with it. + const itemLink = isValidURL(result.link) ? result.link : canonicalURL(result.link, site).href; + xml += `${itemLink}`; + xml += `${itemLink}`; if (result.description) xml += ``; if (result.pubDate) { // note: this should be a Date, but if user provided a string or number, we can work with that, too. @@ -81,13 +83,14 @@ export function generateRSSStylesheet() { } /** Generated function to be run */ -export function generateRssFunction(site: string | undefined, route: RouteData): { generator: RSSFunction; rss?: RSSResult } { - let result: RSSResult = {} as any; +export function generateRssFunction(site: string | undefined, route: RouteData): { generator: RSSFunction; rss?: RSSResult[] } { + let results: RSSResult[] = []; return { generator: function rssUtility(args: RSS) { if (!site) { throw new Error(`[${route.component}] rss() tried to generate RSS but "buildOptions.site" missing in astro.config.mjs`); } + let result: RSSResult = {} as any; const { dest, ...rssData } = args; const feedURL = dest || '/rss.xml'; if (rssData.stylesheet === true) { @@ -105,7 +108,8 @@ export function generateRssFunction(site: string | undefined, route: RouteData): url: feedURL, content: generateRSS({ rssData, site, srcFile: route.component, feedURL }), }; + results.push(result); }, - rss: result, + rss: results, }; } diff --git a/packages/astro/src/core/util.ts b/packages/astro/src/core/util.ts index 10ee44c00..9deb72f5e 100644 --- a/packages/astro/src/core/util.ts +++ b/packages/astro/src/core/util.ts @@ -15,6 +15,15 @@ export function canonicalURL(url: string, base?: string): URL { return new URL(pathname, base); } +/** Check if a URL is already valid */ +export function isValidURL(url: string):boolean { + try { + new URL(url) + return true; + } catch (e) {} + return false; +} + /** is a specifier an npm package? */ export function parseNpmName(spec: string): { scope?: string; name: string; subpath?: string } | undefined { // not an npm package diff --git a/packages/astro/test/astro-sitemap-rss.test.js b/packages/astro/test/astro-sitemap-rss.test.js index f3ef8d220..492c144a2 100644 --- a/packages/astro/test/astro-sitemap-rss.test.js +++ b/packages/astro/test/astro-sitemap-rss.test.js @@ -22,11 +22,18 @@ describe('Sitemaps', () => { const rss = await fixture.readFile('/custom/feed.xml'); expect(rss).to.equal( `<![CDATA[MF Doomcast]]>https://astro.build/custom/feed.xmlen-usMF Doom<![CDATA[Rap Snitch Knishes (feat. Mr. Fantastik)]]>https://astro.build/episode/rap-snitch-knishes/https://astro.build/episode/rap-snitch-knishes/Tue, 16 Nov 2004 00:00:00 GMTmusic172true<![CDATA[Fazers]]>https://astro.build/episode/fazers/https://astro.build/episode/fazers/Thu, 03 Jul 2003 00:00:00 GMTmusic197true<![CDATA[Rhymes Like Dimes (feat. Cucumber Slice)]]>https://astro.build/episode/rhymes-like-dimes/https://astro.build/episode/rhymes-like-dimes/Tue, 19 Oct 1999 00:00:00 GMTmusic259true` + ); + }); + it('generates RSS with pregenerated URLs correctly', async () => { + const rss = await fixture.readFile('/custom/feed-pregenerated-urls.xml'); + expect(rss).to.equal( + `<![CDATA[MF Doomcast]]>https://astro.build/custom/feed-pregenerated-urls.xmlen-usMF Doom<![CDATA[Rap Snitch Knishes (feat. Mr. Fantastik)]]>https://example.com/episode/rap-snitch-knishes/https://example.com/episode/rap-snitch-knishes/Tue, 16 Nov 2004 00:00:00 GMTmusic172true<![CDATA[Fazers]]>https://example.com/episode/fazers/https://example.com/episode/fazers/Thu, 03 Jul 2003 00:00:00 GMTmusic197true<![CDATA[Rhymes Like Dimes (feat. Cucumber Slice)]]>https://example.com/episode/rhymes-like-dimes/https://example.com/episode/rhymes-like-dimes/Tue, 19 Oct 1999 00:00:00 GMTmusic259true` ); }); }); - + describe('Sitemap Generation', () => { it('Generates Sitemap correctly', async () => { let sitemap = await fixture.readFile('/sitemap.xml'); diff --git a/packages/astro/test/fixtures/astro-sitemap-rss/src/pages/episodes/[...page].astro b/packages/astro/test/fixtures/astro-sitemap-rss/src/pages/episodes/[...page].astro index 5cb25e3de..0c0c676ee 100644 --- a/packages/astro/test/fixtures/astro-sitemap-rss/src/pages/episodes/[...page].astro +++ b/packages/astro/test/fixtures/astro-sitemap-rss/src/pages/episodes/[...page].astro @@ -21,6 +21,26 @@ export function getStaticPaths({paginate, rss}) { })), dest: '/custom/feed.xml', }); + rss({ + title: 'MF Doomcast', + description: 'The podcast about the things you find on a picnic, or at a picnic table', + xmlns: { + itunes: 'http://www.itunes.com/dtds/podcast-1.0.dtd', + content: 'http://purl.org/rss/1.0/modules/content/', + }, + customData: `en-us` + + `MF Doom`, + items: episodes.map((episode) => ({ + title: episode.title, + link: `https://example.com${episode.url}/`, + description: episode.description, + pubDate: episode.pubDate + 'Z', + customData: `${episode.type}` + + `${episode.duration}` + + `${episode.explicit || false}`, + })), + dest: '/custom/feed-pregenerated-urls.xml', + }); return paginate(episodes); }