opensource/galaxy
3
0
Fork 0
mirror of https://github.com/galaxyproject/galaxy.git synced 2026-10-16 15:45:04 +08:00
Code Issues Packages Projects Releases Wiki Activity

Tighten the style import check and add tests

Browse Source 
- Flag bare `@import "bootstrap"` and paths into node_modules: Vite
  resolves the bare name through the package's "sass" field to
  scss/bootstrap.scss, so it compiles all of Bootstrap.
- Flag `@import"base"` with no space after the at-rule.
- Resolve the scanned roots relative to the script so it also works when
  run from outside client/.
- Export the matcher and cover it with a vitest test.
- Run the check as part of `make client-lint`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
mvdbeek
2026-10-08 17:50:54 -04:00
committed by Dannon Baker
co-authored by Claude Opus 5.5
parent 0d51cd2b6b
commit e2ce656e79
3 changed files with 78 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Makefile
+4 -1
View File
@@ -220,7 +220,10 @@ client-eslint: client-node-deps # Run client linting
client-format-check: client-node-deps # Run client formatting check
$(IN_VENV) cd client && pnpm run format-check
client-lint: client-eslint client-format-check ## ES lint and check format of client
client-check-style-imports: client-node-deps # Check that components don't import base.scss or all of Bootstrap
$(IN_VENV) cd client && pnpm run check-style-imports
client-lint: client-eslint client-check-style-imports client-format-check ## ES lint and check format of client
client-test-watch: client ## Watch and run all client unit tests on changes
$(IN_VENV) cd client && pnpm run test:watch
client/scripts/check-style-imports.mjs
+35 -25
View File
@@ -2,37 +2,47 @@
// Components must not import stylesheets that emit CSS. In a <style scoped> block every
// selector gets the component's [data-v-*] attribute, so each import compiles a fresh,
// un-dedupable copy of Bootstrap and friends into base.css (see #23703).
import { readdirSync, readFileSync } from "node:fs";
import { readdirSync, readFileSync, realpathSync } from "node:fs";
import { join, relative } from "node:path";
const ROOTS = ["src", "packages/ui/src"];
const FORBIDDEN = [/["'](?:[^"']*\/)?base(?:\.scss)?["']/, /["']~?bootstrap\/scss\/bootstrap(?:\.scss)?["']/];
const IMPORT = /^\s*@(import|use)\s+(.+)$/;
// A bare "bootstrap" also pulls in everything: it resolves through the package's "sass" field.
const FORBIDDEN = [/["'](?:[^"']*\/)?base(?:\.scss)?["']/, /["'](?:[^"']*\/)?~?bootstrap(?:\.scss)?["']/];
const IMPORT = /^\s*@(?:import|use)\b(.+)$/;
const problems = [];
for (const root of ROOTS) {
for (const entry of readdirSync(root, { recursive: true, withFileTypes: true })) {
if (!entry.isFile() || !entry.name.endsWith(".vue")) {
continue;
export function findForbiddenImports(source) {
return source.split("\n").flatMap((line, index) => {
const match = line.match(IMPORT);
return match && FORBIDDEN.some((pattern) => pattern.test(match[1])) ? [`${index + 1}: ${line.trim()}`] : [];
});
}
function main() {
const client = join(import.meta.dirname, "..");
const problems = [];
for (const root of ROOTS) {
for (const entry of readdirSync(join(client, root), { recursive: true, withFileTypes: true })) {
if (!entry.isFile() || !entry.name.endsWith(".vue")) {
continue;
}
const path = join(entry.parentPath, entry.name);
for (const problem of findForbiddenImports(readFileSync(path, "utf8"))) {
problems.push(`${relative(".", path)}:${problem}`);
}
}
const path = join(entry.parentPath, entry.name);
readFileSync(path, "utf8")
.split("\n")
.forEach((line, index) => {
const match = line.match(IMPORT);
if (match && FORBIDDEN.some((pattern) => pattern.test(match[2]))) {
problems.push(`${relative(".", path)}:${index + 1}: ${line.trim()}`);
}
});
}
if (problems.length) {
console.error("Components must not import base.scss or all of Bootstrap:\n");
console.error(problems.map((problem) => ` ${problem}`).join("\n"));
console.error(
'\nEach import adds a full scoped copy (~600 KB) to base.css. Import "@/style/scss/theme/blue.scss"' +
"\nfor variables instead.",
);
process.exit(1);
}
}
if (problems.length) {
console.error("Components must not import base.scss or all of Bootstrap:\n");
console.error(problems.map((problem) => ` ${problem}`).join("\n"));
console.error(
'\nEach import adds a full scoped copy (~600 KB) to base.css. Import "@/style/scss/theme/blue.scss"' +
"\nfor variables instead.",
);
process.exit(1);
if (process.argv[1] && realpathSync(process.argv[1]) === import.meta.filename) {
main();
}
client/tests/vitest/check-style-imports.test.js
+39
View File
@@ -0,0 +1,39 @@
import { describe, expect, it } from "vitest";
import { findForbiddenImports } from "../../scripts/check-style-imports.mjs";
function component(styleImport) {
return `<template><div /></template>\n<style scoped lang="scss">\n${styleImport}\n</style>\n`;
}
describe("check-style-imports", () => {
it.each([
'@import "base";',
'@import "base.scss";',
'@import "@/style/scss/base.scss";',
'@import "../../style/scss/base";',
"@use 'base';",
'@import"base";',
'@import "theme/blue.scss", "base";',
'@import "bootstrap";',
'@import "bootstrap/scss/bootstrap";',
'@import "bootstrap/scss/bootstrap.scss";',
'@import "~bootstrap/scss/bootstrap";',
'@import "../../../node_modules/bootstrap/scss/bootstrap";',
])("flags %s", (styleImport) => {
expect(findForbiddenImports(component(styleImport))).toEqual([`3: ${styleImport}`]);
});
it.each([
'@import "@/style/scss/theme/blue.scss";',
'@import "bootstrap/scss/functions";',
'@import "bootstrap/scss/_functions.scss";',
'@import "bootstrap/scss/mixins";',
'@import "bootstrap-vue/src/index.scss";',
'@import "@/components/Form/_form-elements.scss";',
'@import "x/database";',
'// @import "base";',
])("allows %s", (styleImport) => {
expect(findForbiddenImports(component(styleImport))).toEqual([]);
});
});