From b3d936ac248c0b939ff830023d75694398094341 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 10 Nov 2022 14:12:42 -0800 Subject: [PATCH] Updated CSS naming algorithm (#5353) * Updated CSS naming algorithm * Adding a changeset * Fix windows --- .changeset/shiny-baboons-trade.md | 5 + .../astro/src/core/build/css-asset-name.ts | 77 +++++++++++++ .../astro/src/core/build/vite-plugin-css.ts | 23 +--- .../astro/test/astro-css-bundling.test.js | 105 +++++++++++++----- 4 files changed, 165 insertions(+), 45 deletions(-) create mode 100644 .changeset/shiny-baboons-trade.md create mode 100644 packages/astro/src/core/build/css-asset-name.ts diff --git a/.changeset/shiny-baboons-trade.md b/.changeset/shiny-baboons-trade.md new file mode 100644 index 000000000000..e64c8d024213 --- /dev/null +++ b/.changeset/shiny-baboons-trade.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Updated the CSS naming algorithm to prevent clashes diff --git a/packages/astro/src/core/build/css-asset-name.ts b/packages/astro/src/core/build/css-asset-name.ts new file mode 100644 index 000000000000..1148cb9aa69c --- /dev/null +++ b/packages/astro/src/core/build/css-asset-name.ts @@ -0,0 +1,77 @@ +import type { GetModuleInfo } from 'rollup'; + +import crypto from 'crypto'; +import npath from 'path'; +import { getTopLevelPages } from './graph.js'; +import { AstroSettings } from '../../@types/astro'; +import { viteID } from '../util.js'; + +// The short name for when the hash can be included +// We could get rid of this and only use the createSlugger implementation, but this creates +// slightly prettier names. +export function shortHashedName(id: string, ctx: { getModuleInfo: GetModuleInfo }): string { + const parents = Array.from(getTopLevelPages(id, ctx)); + const firstParentId = parents[0]?.[0].id; + const firstParentName = firstParentId ? npath.parse(firstParentId).name : 'index'; + + const hash = crypto.createHash('sha256'); + for (const [page] of parents) { + hash.update(page.id, 'utf-8'); + } + const h = hash.digest('hex').slice(0, 8); + const proposedName = firstParentName + '.' + h; + return proposedName; +} + +export function createSlugger(settings: AstroSettings) { + const pagesDir = viteID(new URL('./pages', settings.config.srcDir)); + const map = new Map>(); + const sep = '-'; + return function(id: string, ctx: { getModuleInfo: GetModuleInfo }): string { + const parents = Array.from(getTopLevelPages(id, ctx)); + const allParentsKey = parents + .map(([page]) => page.id) + .sort() + .join('-'); + const firstParentId = parents[0]?.[0].id || 'index'; + + // Use the last two segments, for ex /docs/index + let dir = firstParentId; + let key = ''; + let i = 0; + while (i < 2) { + if(dir === pagesDir) { + break; + } + + const name = npath.parse(npath.basename(dir)).name; + key = key.length ? (name + sep + key) : name; + dir = npath.dirname(dir); + i++; + } + + // Keep track of how many times this was used. + let name = key; + + // The map keeps track of how many times a key, like `pages_index` is used as the name. + // If the same key is used more than once we increment a number so it becomes `pages-index-1`. + // This guarantees that it stays unique, without sacrificing pretty names. + if(!map.has(key)) { + map.set(key, new Map([[allParentsKey, 0]])); + } else { + const inner = map.get(key)!; + if(inner.has(allParentsKey)) { + const num = inner.get(allParentsKey)!; + if(num > 0) { + name = name + sep + num; + } + } else { + const num = inner.size; + inner.set(allParentsKey, num); + name = name + sep + num; + } + } + + return name; + }; +} diff --git a/packages/astro/src/core/build/vite-plugin-css.ts b/packages/astro/src/core/build/vite-plugin-css.ts index 347786ea5340..881870f88e32 100644 --- a/packages/astro/src/core/build/vite-plugin-css.ts +++ b/packages/astro/src/core/build/vite-plugin-css.ts @@ -2,12 +2,11 @@ import type { GetModuleInfo } from 'rollup'; import type { BuildInternals } from './internal'; import type { PageBuildData, StaticBuildOptions } from './types'; -import crypto from 'crypto'; import esbuild from 'esbuild'; -import npath from 'path'; import { Plugin as VitePlugin, ResolvedConfig } from 'vite'; import { isCSSRequest } from '../render/util.js'; -import { getTopLevelPages, moduleIsTopLevelPage, walkParentInfos } from './graph.js'; + +import { moduleIsTopLevelPage, walkParentInfos } from './graph.js'; import { eachPageData, getPageDataByViteID, @@ -15,6 +14,7 @@ import { getPageDatasByHoistedScriptId, isHoistedScript, } from './internal.js'; +import * as assetName from './css-asset-name.js'; interface PluginOptions { internals: BuildInternals; @@ -28,20 +28,6 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] let resolvedConfig: ResolvedConfig; - function createNameForParentPages(id: string, ctx: { getModuleInfo: GetModuleInfo }): string { - const parents = Array.from(getTopLevelPages(id, ctx)); - const firstParentId = parents[0]?.[0].id; - const firstParentName = firstParentId ? npath.parse(firstParentId).name : 'index'; - - const hash = crypto.createHash('sha256'); - for (const [page] of parents) { - hash.update(page.id, 'utf-8'); - } - const h = hash.digest('hex').slice(0, 8); - const proposedName = firstParentName + '.' + h; - return proposedName; - } - function* getParentClientOnlys( id: string, ctx: { getModuleInfo: GetModuleInfo } @@ -57,6 +43,9 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] outputOptions(outputOptions) { const manualChunks = outputOptions.manualChunks || Function.prototype; + const assetFileNames = outputOptions.assetFileNames; + const namingIncludesHash = assetFileNames?.toString().includes('[hash]'); + const createNameForParentPages = namingIncludesHash ? assetName.shortHashedName : assetName.createSlugger(settings); outputOptions.manualChunks = function (id, ...args) { // Defer to user-provided `manualChunks`, if it was provided. if (typeof manualChunks == 'object') { diff --git a/packages/astro/test/astro-css-bundling.test.js b/packages/astro/test/astro-css-bundling.test.js index 1c470c0ef55b..ec80e75dc21e 100644 --- a/packages/astro/test/astro-css-bundling.test.js +++ b/packages/astro/test/astro-css-bundling.test.js @@ -19,42 +19,91 @@ const UNEXPECTED_CSS = [ ]; describe('CSS Bundling', function () { + /** @type {import('./test-utils').Fixture} */ let fixture; - before(async () => { - fixture = await loadFixture({ - root: './fixtures/astro-css-bundling/', + describe('defaults', () => { + before(async () => { + fixture = await loadFixture({ + root: './fixtures/astro-css-bundling/', + }); + await fixture.build({ mode: 'production' }); + }); + + it('Bundles CSS', async () => { + const builtCSS = new Set(); + + // for all HTML files… + for (const [filepath, css] of Object.entries(EXPECTED_CSS)) { + const html = await fixture.readFile(filepath); + const $ = cheerio.load(html); + + // test 1: assert new bundled CSS is present + for (const href of css) { + const link = $(`link[rel="stylesheet"][href^="${href}"]`); + expect(link.length).to.be.greaterThanOrEqual(1); + const outHref = link.attr('href'); + builtCSS.add(outHref.startsWith('../') ? outHref.slice(2) : outHref); + } + + // test 2: assert old CSS was removed + for (const href of UNEXPECTED_CSS) { + const link = $(`link[rel="stylesheet"][href="${href}"]`); + expect(link).to.have.lengthOf(0); + } + + // test 3: assert all bundled CSS was built and contains CSS + for (const url of builtCSS.keys()) { + const bundledCss = await fixture.readFile(url); + expect(bundledCss).to.be.ok; + } + } + }); + + it('there are 4 css files', async () => { + const dir = await fixture.readdir('/assets'); + expect(dir).to.have.a.lengthOf(4); + }); + + it('CSS includes hashes', async () => { + const [firstFound] = await fixture.readdir('/assets'); + expect(firstFound).to.match(/[a-z]+\.[0-9a-z]{8}\.css/); }); - await fixture.build({ mode: 'production' }); }); - it('Bundles CSS', async () => { - const builtCSS = new Set(); + describe('using custom assetFileNames config', () => { + before(async () => { + fixture = await loadFixture({ + root: './fixtures/astro-css-bundling/', - // for all HTML files… - for (const [filepath, css] of Object.entries(EXPECTED_CSS)) { - const html = await fixture.readFile(filepath); - const $ = cheerio.load(html); + vite: { + build: { + rollupOptions: { + output: { + assetFileNames: "assets/[name][extname]", + entryFileNames: "[name].js", + } + } + } + } + }); + await fixture.build({ mode: 'production' }); + }); - // test 1: assert new bundled CSS is present - for (const href of css) { - const link = $(`link[rel="stylesheet"][href^="${href}"]`); - expect(link.length).to.be.greaterThanOrEqual(1); - const outHref = link.attr('href'); - builtCSS.add(outHref.startsWith('../') ? outHref.slice(2) : outHref); - } + it('there are 4 css files', async () => { + const dir = await fixture.readdir('/assets'); + expect(dir).to.have.a.lengthOf(4); + }); - // test 2: assert old CSS was removed - for (const href of UNEXPECTED_CSS) { - const link = $(`link[rel="stylesheet"][href="${href}"]`); - expect(link).to.have.lengthOf(0); - } + it('CSS does not include hashes hashes', async () => { + const [firstFound] = await fixture.readdir('/assets'); + expect(firstFound).to.not.match(/[a-z]+\.[0-9a-z]{8}\.css/); + }); - // test 3: assert all bundled CSS was built and contains CSS - for (const url of builtCSS.keys()) { - const bundledCss = await fixture.readFile(url); - expect(bundledCss).to.be.ok; - } - } + it('there are 2 index named CSS files', async () => { + const dir = await fixture.readdir('/assets'); + const indexNamedFiles = dir.filter(name => name.startsWith('index')) + expect(indexNamedFiles).to.have.a.lengthOf(2); + }); }); });