From dd92871fd77face243b51be29bd7f675f985d6e0 Mon Sep 17 00:00:00 2001 From: Bartek Igielski Date: Fri, 10 Sep 2021 19:17:17 +0200 Subject: [PATCH] Prevent removing CSS preloads during bundling (#1326) * Prevent removing nodes, becasue styles preloading was detected earlier * Add separate deduping for preloads and cover it with tests. * Create quiet-horses-turn.md * Test merging preload tags --- .changeset/quiet-horses-turn.md | 5 +++ packages/astro/src/build/bundle/css.ts | 41 +++++++++++++------ .../astro/test/astro-css-bundling.test.js | 34 +++++++++++---- .../src/css/page-preload-merge-2.css | 3 ++ .../src/css/page-preload-merge.css | 3 ++ .../src/css/page-preload.css | 3 ++ .../src/pages/preload-merge.astro | 17 ++++++++ .../src/pages/preload.astro | 14 +++++++ 8 files changed, 99 insertions(+), 21 deletions(-) create mode 100644 .changeset/quiet-horses-turn.md create mode 100644 packages/astro/test/fixtures/astro-css-bundling/src/css/page-preload-merge-2.css create mode 100644 packages/astro/test/fixtures/astro-css-bundling/src/css/page-preload-merge.css create mode 100644 packages/astro/test/fixtures/astro-css-bundling/src/css/page-preload.css create mode 100644 packages/astro/test/fixtures/astro-css-bundling/src/pages/preload-merge.astro create mode 100644 packages/astro/test/fixtures/astro-css-bundling/src/pages/preload.astro diff --git a/.changeset/quiet-horses-turn.md b/.changeset/quiet-horses-turn.md new file mode 100644 index 000000000..bd4aab328 --- /dev/null +++ b/.changeset/quiet-horses-turn.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +During CSS bundling separate processing of `rel="preload"` from normal loading stylesheets, to preserve preloads, and source element attributes like `media`. diff --git a/packages/astro/src/build/bundle/css.ts b/packages/astro/src/build/bundle/css.ts index 6da84da02..e585ce393 100644 --- a/packages/astro/src/build/bundle/css.ts +++ b/packages/astro/src/build/bundle/css.ts @@ -115,23 +115,40 @@ export async function bundleCSS({ if (buildState[id].contentType !== 'text/html') return; const $ = cheerio.load(buildState[id].contents); - const pageCSS = new Set(); // keep track of page-specific CSS so we remove dupes + const stylesheets = new Set(); // keep track of page-specific CSS so we remove dupes + const preloads = new Set(); // list of stylesheets preloads, to remove dupes + $('link[href]').each((i, el) => { const srcPath = getSrcPath(id, { astroConfig }); const oldHref = getDistPath($(el).attr('href') || '', { astroConfig, srcPath }); // note: this may be a relative URL; transform to absolute to find a buildOutput match const newHref = cssMap.get(oldHref); - if (newHref) { - // note: link[href] will select too much, however, remote CSS and non-CSS link tags won’t be in cssMap - if (pageCSS.has(newHref)) { - $(el).remove(); // this is a dupe; remove - } else { - $(el).attr('href', cssHashes.get(newHref) || ''); // new CSS; update href (important! use cssHashes, not cssMap) - pageCSS.add(newHref); - } - // bonus: add [rel] and [type]. not necessary, but why not? - $(el).attr('rel', 'stylesheet'); - $(el).attr('type', 'text/css'); + + if (!newHref) { + return } + + if (el.attribs?.rel === 'preload') { + if (preloads.has(newHref)) { + $(el).remove(); + } else { + $(el).attr("href", cssHashes.get(newHref) || ""); + preloads.add(newHref); + } + return + } + + if (stylesheets.has(newHref)) { + $(el).remove(); // this is a dupe; remove + } else { + $(el).attr("href", cssHashes.get(newHref) || ""); // new CSS; update href (important! use cssHashes, not cssMap) + + // bonus: add [rel] and [type]. not necessary, but why not? + $(el).attr("rel", "stylesheet"); + $(el).attr("type", "text/css"); + + stylesheets.add(newHref); + } + }); (buildState[id] as any).contents = $.html(); // save updated HTML in global buildState }) diff --git a/packages/astro/test/astro-css-bundling.test.js b/packages/astro/test/astro-css-bundling.test.js index d674c6657..a69fb69dd 100644 --- a/packages/astro/test/astro-css-bundling.test.js +++ b/packages/astro/test/astro-css-bundling.test.js @@ -13,6 +13,8 @@ const EXPECTED_CSS = { '/index.html': ['/_astro/common-', '/_astro/index-'], // don’t match hashes, which change based on content '/one/index.html': ['/_astro/common-', '/_astro/one/index-'], '/two/index.html': ['/_astro/common-', '/_astro/two/index-'], + '/preload/index.html': ['/_astro/common-', '/_astro/preload/index-'], + '/preload-merge/index.html': ['/_astro/preload-merge/index-'] }; const UNEXPECTED_CSS = ['/_astro/components/nav.css', '../css/typography.css', '../css/colors.css', '../css/page-index.css', '../css/page-one.css', '../css/page-two.css']; @@ -28,40 +30,54 @@ CSSBundling('Bundles CSS', async (context) => { // test 1: assert new bundled CSS is present for (const href of css) { - const link = $(`link[href^="${href}"]`); - assert.equal(link.length, 1); + const link = $(`link[rel="stylesheet"][href^="${href}"]`); + assert.equal(link.length, 1, 'New bundled CSS is not present'); builtCSS.add(link.attr('href')); } // test 2: assert old CSS was removed for (const href of UNEXPECTED_CSS) { - const link = $(`link[href="${href}"]`); - assert.equal(link.length, 0); + const link = $(`link[rel="stylesheet"][href="${href}"]`); + assert.equal(link.length, 0, 'Old CSS was not removed'); + } + + // test 3: preload tags was not removed and attributes was preserved + if (filepath === '/preload/index.html') { + const stylesheet = $('link[rel="stylesheet"][href^="/_astro/preload/index-"]'); + const preload = $('link[rel="preload"][href^="/_astro/preload/index-"]'); + assert.equal(stylesheet[0].attribs.media, 'print', 'Attribute was not preserved'); + assert.equal(preload.length, 1, 'Preload tag was removed'); + } + + // test 4: preload tags was not removed and attributes was preserved + if (filepath === '/preload-merge/index.html') { + const preload = $('link[rel="preload"]'); + assert.equal(preload.length, 1, 'Preload tag was not merged or was removed completly'); } } - // test 3: assert all bundled CSS was built and contains CSS + // test 5: assert all bundled CSS was built and contains CSS for (const url of builtCSS.keys()) { const css = await context.readFile(url); assert.ok(css, true); } - // test 4: assert ordering is preserved (typography.css before colors.css) + // test 6: assert ordering is preserved (typography.css before colors.css) const bundledLoc = [...builtCSS].find((k) => k.startsWith('/_astro/common-')); const bundledContents = await context.readFile(bundledLoc); const typographyIndex = bundledContents.indexOf('body{'); const colorsIndex = bundledContents.indexOf(':root{'); assert.ok(typographyIndex < colorsIndex); - // test 5: assert multiple style blocks were bundled (Nav.astro includes 2 scoped style blocks) + // test 7: assert multiple style blocks were bundled (Nav.astro includes 2 scoped style blocks) const scopedNavStyles = [...bundledContents.matchAll('.nav.astro-')]; assert.is(scopedNavStyles.length, 2); - // test 6: assert