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

Make CSS chunk names less confusing #8754

merged 2 commits into from Oct 9, 2023

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Oct 5, 2023

Changes

fix #7469

This PR implements a lame algorithm to pick a chunk base name that is not confusing. Simply by avoiding the 404 and 500 names if there's others available.

I couldn't quite figure out a better naming-scheme without overcomplicating it. I also tried resorting this to Rollup, but there's edgecases where that's not possible. Our CSS handling that links CSS to HTML pages are very ingrained throughout the codebase. Also in practice, Rollup would also pick the confusing name otherwise.

Testing

Tested manually.

Docs

n/a.

@changeset-bot
Copy link

changeset-bot bot commented Oct 5, 2023

🦋 Changeset detected

Latest commit: ee00eec

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Oct 5, 2023

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).

@lilnasy
Copy link
Contributor

lilnasy commented Oct 5, 2023

I couldn't quite figure out a better naming-scheme without overcomplicating it.

Do we need to include a real file's name at all? Could we let it be just the hash?

@bluwy
Copy link
Member Author

bluwy commented Oct 5, 2023

I think we still need some sort of memorable name so it's easier to debug things when needed.

@bluwy bluwy merged commit 93b0922 into main Oct 9, 2023
13 checks passed
@bluwy bluwy deleted the css-not-confusing branch October 9, 2023 09:29
@astrobot-houston astrobot-houston mentioned this pull request Oct 9, 2023
@Michael0520 Michael0520 mentioned this pull request Dec 17, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strange naming of css files
4 participants