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.

Sign up for GitHub

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

Jump to bottom

fix(cli): the internal order of the parts of the built css files was nondeterministic #2560

Merged
merged 2 commits into from
Oct 3, 2024
+44 −5
Merged

fix(cli): the internal order of the parts of the built css files was nondeterministic #2560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5e80adc
fix(cli): the internal order of the parts of the built css files was …
unekinn Oct 2, 2024
1c5980f
Add changeset
unekinn Oct 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/yellow-zoos-camp.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@digdir/designsystemet': patch
---

Make sure the internal order of sections in the CSS generated by the CLI is deterministic, to avoid unnecessary git diffs
44 changes: 39 additions & 5 deletions packages/cli/src/tokens/build/utils/entryfile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,49 @@ import glob from 'fast-glob';
import fs from 'fs-extra';
import * as R from 'ramda';

const sortLightmodeFirst = R.sortWith<string>([R.descend(R.includes('light')), R.descend(R.includes('secondary'))]);
/**
* Defines a sort order for the sections of the entry CSS file.
* This ensures a deterministic order, whereas earlier this was nondeterministic
*/
// biome-ignore format: keep array as one line per item
const sortOrder = [
'color-mode/light',
'typography/secondary',
'semantic',
'color-mode/dark',
'color-mode/contrast',
'typography/primary',
];
Comment on lines +12 to +19
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This sort order keeps the brand css files unchanged from the existing commited files when running yarn build:tokens. But is there a reason for this exact ordering? The old sorting function would be equivalent to this:

const sortOrder = [
  'color-mode/light',
  'typography/secondary',
];

That is to say, these two would always come first, and then the rest in an unspecified — but usually the same — order.

However, wouldn't it make more sense for the things that are added to :root -- color-mode/light, typography/primary, and semantic, to be first, and then other color modes, then typography/secondary?

Any thoughts @Barsnes & @eirikbacker

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest we create an issue of this and can as Mickey when he is back, and merge the rest of the changes - nice work 👏 🫶

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#2562


const sortByDefinedOrder = R.sortBy<string>((fileName) => {
const sortIndex = sortOrder.findIndex((sortElement) => fileName.includes(`${sortElement}.css`));
if (sortIndex === -1) {
// Ensure file names that don't have a specified sort order appear last
console.error(
chalk.yellow('WARNING: CSS section does not have a defined sort order:', fileName.replace('.css', '')),
);
console.log(
chalk.dim(
`
A Digdir developer should define its order in the sortOrder array in entryfile.ts.
The section will currently be added to the end of the entry file, but the exact
order may change due to nondeterminism.`.trim(),
),
);
console.log();

return Infinity;
}
return sortIndex;
});

const header = `@charset "UTF-8";

@layer ds.reset, ds.theme, ds.base, ds.utilities, ds.components;
\n`;

const sortAndConcat = R.pipe(
sortLightmodeFirst,
R.map((file): string => {
const concat = R.pipe(
R.map((file: string): string => {
try {
const content = fs.readFileSync(file, 'utf-8').toString();
return content;
Expand All @@ -38,7 +71,8 @@ export const makeEntryFile: EntryFile = async ({ outPath, buildPath, theme }) =>
const writePath = `${outPath}/${theme}.css`;

const files = await glob(`**/*`, { cwd: buildPath });
const content = header + sortAndConcat(files.map((file) => `${buildPath}/${file}`));
const sortedFileNames = sortByDefinedOrder(files);
const content = header + concat(sortedFileNames.map((file) => `${buildPath}/${file}`));

await fs.writeFile(writePath, content);
};
Loading