Skip to content

Commit

Permalink
Updated CSS naming algorithm (#5353)
Browse files Browse the repository at this point in the history 
* Updated CSS naming algorithm

* Adding a changeset

* Fix windows
  • Loading branch information
@matthewp
matthewp authored Nov 10, 2022
1 parent f811518 commit b3d936a
Show file tree
Hide file tree
Showing 4 changed files with 165 additions and 45 deletions.
5 changes: 5 additions & 0 deletions .changeset/shiny-baboons-trade.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Updated the CSS naming algorithm to prevent clashes
77 changes: 77 additions & 0 deletions packages/astro/src/core/build/css-asset-name.ts
View file
Original file line number Diff line number Diff line change
@@ -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<string, Map<string, number>>();
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;
};
}
23 changes: 6 additions & 17 deletions packages/astro/src/core/build/vite-plugin-css.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,19 @@ 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,
getPageDatasByClientOnlyID,
getPageDatasByHoistedScriptId,
isHoistedScript,
} from './internal.js';
import * as assetName from './css-asset-name.js';

interface PluginOptions {
internals: BuildInternals;
Expand All @@ -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 }
Expand All @@ -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') {
Expand Down
105 changes: 77 additions & 28 deletions packages/astro/test/astro-css-bundling.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});

0 comments on commit b3d936a

Please sign in to comment.