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

Make CSS chunk names less confusing #8754

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/sharp-insects-yawn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Make CSS chunk names less confusing
54 changes: 46 additions & 8 deletions packages/astro/src/core/build/css-asset-name.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,34 @@
import type { GetModuleInfo } from 'rollup';
import type { GetModuleInfo, ModuleInfo } from 'rollup';

import crypto from 'node:crypto';
import npath from 'node:path';
import type { AstroSettings } from '../../@types/astro.js';
import { viteID } from '../util.js';
import { getTopLevelPages } from './graph.js';

// These pages could be used as base names for the chunk hashed name, but they are confusing
// and should be avoided it possible
const confusingBaseNames = ['404', '500'];

Copy link
Member

Choose a reason for hiding this comment

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

You might want to avoid dynamic routes as well, for instance, my website's CSS file is named _...page_.1d4448e7.css (with the underscore at the start, weirdly enough)

Copy link
Member Author

Choose a reason for hiding this comment

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

I could perhaps look into prettify some names too 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated it so . is converted to _, and multiple _ are collapsed to a single _. So _...page_.1d4448e7.css should look like _page_.1d4448e7.css now. There could be same names if there's [slug] and [...slug] side-by-side, but I think it should be fine (hashes are still different so chunking is still correct).

// 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';
return createNameHash(
getFirstParentId(parents),
parents.map(([page]) => page.id)
);
}

export function createNameHash(baseId: string | undefined, hashIds: string[]): string {
const baseName = baseId ? prettifyBaseName(npath.parse(baseId).name) : 'index';
const hash = crypto.createHash('sha256');
for (const [page] of parents) {
hash.update(page.id, 'utf-8');
for (const id of hashIds) {
hash.update(id, 'utf-8');
}
const h = hash.digest('hex').slice(0, 8);
const proposedName = firstParentName + '.' + h;
const proposedName = baseName + '.' + h;
return proposedName;
}

Expand All @@ -34,7 +43,7 @@ export function createSlugger(settings: AstroSettings) {
.map(([page]) => page.id)
.sort()
.join('-');
const firstParentId = parents[0]?.[0].id || indexPage;
const firstParentId = getFirstParentId(parents) || indexPage;

// Use the last two segments, for ex /docs/index
let dir = firstParentId;
Expand All @@ -45,7 +54,7 @@ export function createSlugger(settings: AstroSettings) {
break;
}

const name = npath.parse(npath.basename(dir)).name;
const name = prettifyBaseName(npath.parse(npath.basename(dir)).name);
key = key.length ? name + sep + key : name;
dir = npath.dirname(dir);
i++;
Expand Down Expand Up @@ -76,3 +85,32 @@ export function createSlugger(settings: AstroSettings) {
return name;
};
}

/**
* Find the first parent id from `parents` where its name is not confusing.
* Returns undefined if there's no parents.
*/
function getFirstParentId(parents: [ModuleInfo, number, number][]) {
for (const parent of parents) {
const id = parent[0].id;
const baseName = npath.parse(id).name;
if (!confusingBaseNames.includes(baseName)) {
return id;
}
}
// If all parents are confusing, just use the first one. Or if there's no
// parents, this will return undefined.
return parents[0]?.[0].id;
}

const charsToReplaceRe = /[.\[\]]/g;
const underscoresRe = /_+/g;
/**
* Prettify base names so they're easier to read:
* - index -> index
* - [slug] -> _slug_
* - [...spread] -> _spread_
*/
function prettifyBaseName(str: string) {
return str.replace(charsToReplaceRe, '_').replace(underscoresRe, '_');
}
15 changes: 1 addition & 14 deletions packages/astro/src/core/build/plugins/plugin-css.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import * as crypto from 'node:crypto';
import * as npath from 'node:path';
import type { GetModuleInfo } from 'rollup';
import { type ResolvedConfig, type Plugin as VitePlugin } from 'vite';
import { isBuildableCSSRequest } from '../../../vite-plugin-astro-server/util.js';
Expand Down Expand Up @@ -93,7 +91,7 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {
if (new URL(pageInfo.id, 'file://').searchParams.has(PROPAGATED_ASSET_FLAG)) {
// Split delayed assets to separate modules
// so they can be injected where needed
const chunkId = createNameHash(id, [id]);
const chunkId = assetName.createNameHash(id, [id]);
internals.cssModuleToChunkIdMap.set(id, chunkId);
return chunkId;
}
Expand Down Expand Up @@ -272,17 +270,6 @@ function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[] {

/***** UTILITY FUNCTIONS *****/

function createNameHash(baseId: string, hashIds: string[]): string {
const baseName = baseId ? npath.parse(baseId).name : 'index';
const hash = crypto.createHash('sha256');
for (const id of hashIds) {
hash.update(id, 'utf-8');
}
const h = hash.digest('hex').slice(0, 8);
const proposedName = baseName + '.' + h;
return proposedName;
}

function* getParentClientOnlys(
id: string,
ctx: { getModuleInfo: GetModuleInfo },
Expand Down