From 5bcc03e8f8175e460b446c40d60736dab758080c Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Wed, 5 Aug 2026 21:44:20 +0900 Subject: [PATCH] fix(build): re-link component stylesheets so consumers can load them 0.1.0-alpha.0 shipped every component unstyled. In library mode Rollup strips `import "./Button.css"` out of the chunk and leaves an "empty css" marker, because an application build would have injected a link tag instead. Nothing put the import back, so all 20 component stylesheets were packed under `dist/assets/` with no chunk importing them and no exports entry naming them. A consumer had no supported way to load them at all: `assets/*` is not in the exports map, so a deep import fails with ERR_PACKAGE_PATH_NOT_EXPORTED, and `styles/base.css` carries only the 113 tokens. Measured against the install fixture: the published tarball produces a 4.52 kB consumer bundle (tokens only) where this build produces 21.06 kB. `linkComponentStyles` re-attaches each emitted stylesheet to the chunk whose module imported it, which is what makes README's "component CSS travels with the component" true and leaves every call site unchanged. It reads the module graph rather than pairing `Button.css` with `Button.tsx` by filename, since that convention holds for every component today and would break silently on the first one that departs from it. Imports are appended, not prepended: ES modules hoist them, so execution order is unchanged while existing lines keep their positions and the sourcemap stays accurate. Both existing checks were green throughout the defect, because each asked only whether advertised paths resolve, never whether a shipped file was reachable. Two guards close that: - `check:pack` now fails on a packed stylesheet that no module imports and no exports entry names. Against the alpha.0 build it reports all 20. - CI asserts the fixture's own bundle carries the rules, using markers read out of the packed stylesheet so a class rename is not a false failure. Against the published tarball it fails on Button and Drawer, one per import shape. --- .github/workflows/ci.yml | 6 +++ CHANGELOG.md | 24 ++++++++- README.md | 7 ++- package.json | 2 +- scripts/check-fixture-styles.mjs | 71 +++++++++++++++++++++++++ scripts/check-pack.mjs | 74 ++++++++++++++++++++++---- vite.config.ts | 91 +++++++++++++++++++++++++++++++- 7 files changed, 260 insertions(+), 15 deletions(-) create mode 100644 scripts/check-fixture-styles.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3025953..8284e8e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -86,3 +86,9 @@ jobs: run: | npx tsc --noEmit npx vite build + + # Building is not the same as being styled. 0.1.0-alpha.0 built here + # green while shipping every component's CSS as an asset nothing could + # reach, so this asserts the rules actually land in the consumer bundle. + - name: Confirm component styles reached the consumer + run: node scripts/check-fixture-styles.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 488cc3b..5c61c0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,32 @@ Versioning follows the policy in [CONTRIBUTING.md](CONTRIBUTING.md#versioning). ## [Unreleased] +## [0.1.0-alpha.1] + +### Fixed + +- Component stylesheets now reach the consumer. In library mode Rollup strips + `import "./Button.css"` out of the chunk and nothing puts it back, so + 0.1.0-alpha.0 shipped all 20 component stylesheets as emitted assets that no + chunk imported and no `exports` entry named. Every component rendered + unstyled and there was no supported way to load the rules: a consumer + bundling `Button` got 4.52 kB of CSS (tokens only) where it should have got + 21.06 kB. The build now re-attaches each stylesheet to the chunk whose module + imported it, so nothing changes at the call site. +- `check:pack` now fails on a packed stylesheet that no module imports and no + `exports` entry names, and CI asserts that a component's rules land in the + install fixture's own bundle. Both checks were green across the defect + because each only asked whether advertised paths resolve, never whether a + shipped file was reachable. + +## [0.1.0-alpha.0] + ### Added - Repository bootstrap: build, type-check, lint, format, test, packed-artifact validation, and a clean external React install fixture. - Apache-2.0 license and the initial public boundary rules. -[Unreleased]: https://github.com/lablup/ui-common/compare/main...HEAD +[Unreleased]: https://github.com/lablup/ui-common/compare/v0.1.0-alpha.1...HEAD +[0.1.0-alpha.1]: https://github.com/lablup/ui-common/compare/v0.1.0-alpha.0...v0.1.0-alpha.1 +[0.1.0-alpha.0]: https://github.com/lablup/ui-common/releases/tag/v0.1.0-alpha.0 diff --git a/README.md b/README.md index 2a074aa..df1fbf1 100644 --- a/README.md +++ b/README.md @@ -71,8 +71,11 @@ import "@lablup/ui-common/styles/base.css"; import "@lablup/ui-common/styles/themes/bliss-light.css"; ``` -Component CSS travels with the component, so a subpath import pulls only that -component's styles. +Component CSS travels with the component: importing `Button` brings +`Button.css` with it, so a subpath import pulls that component's styles and no +others. The two entry points above are the only stylesheets you import by hand, +and `base.css` is the one you must not skip, since it carries the tokens every +component resolves against. ## Text is yours, not ours diff --git a/package.json b/package.json index 7b996ef..08cc307 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@lablup/ui-common", - "version": "0.1.0-alpha.0", + "version": "0.1.0-alpha.1", "description": "Shared, product-neutral UI components and design tokens for Lablup products", "license": "Apache-2.0", "author": "Lablup Inc.", diff --git a/scripts/check-fixture-styles.mjs b/scripts/check-fixture-styles.mjs new file mode 100644 index 0000000..b8ff7c6 --- /dev/null +++ b/scripts/check-fixture-styles.mjs @@ -0,0 +1,71 @@ +#!/usr/bin/env node +/** + * Consumer-side stylesheet check. + * + * `check-pack.mjs` asserts that every packed stylesheet is reachable. This + * asserts the consequence a user actually experiences: a project that installs + * the tarball and imports a component ends up with that component's rules in + * its own bundle. Run against the built install fixture, after `vite build`. + * + * Two components, because there are two import shapes and each could break on + * its own: `Button` arrives through the package root, `Drawer` through a + * component subpath. + * + * Markers are read out of the packed stylesheet rather than written down here, + * so renaming a class is not a false failure. + */ +import { readdir, readFile } from "node:fs/promises"; +import { fileURLToPath } from "node:url"; +import { dirname, join, resolve } from "node:path"; + +const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +const fixtureDist = resolve(root, process.argv[2] ?? "fixture/dist"); + +const COMPONENTS = ["Button", "Drawer"]; + +async function collectCss(dir) { + const out = []; + for (const entry of await readdir(dir, { withFileTypes: true })) { + const path = join(dir, entry.name); + if (entry.isDirectory()) out.push(...(await collectCss(path))); + else if (entry.name.endsWith(".css")) out.push(await readFile(path, "utf8")); + } + return out; +} + +/** The first class selector in a stylesheet, e.g. ".button" out of ".button{...". */ +function firstClassSelector(css, source) { + const match = css.match(/\.(-?[_a-zA-Z][\w-]*)/); + if (!match) throw new Error(`${source} defines no class selector to check against`); + return match[0]; +} + +const bundled = (await collectCss(fixtureDist)).join("\n"); +if (bundled.trim() === "") { + console.error(`No stylesheet found under ${fixtureDist}. Build the fixture first.`); + process.exit(1); +} + +const missing = []; + +for (const component of COMPONENTS) { + const source = `dist/assets/components/${component}/${component}.css`; + const marker = firstClassSelector( + await readFile(resolve(root, source), "utf8"), + source, + ); + if (!bundled.includes(marker)) { + missing.push( + `the fixture imports ${component} but its bundle carries no "${marker}" rule, ` + + `so ${source} never reached the consumer`, + ); + } +} + +if (missing.length > 0) { + console.error(`Consumer stylesheet check failed (${missing.length}):\n`); + for (const line of missing) console.error(` ${line}`); + process.exit(1); +} + +console.log(`Consumer stylesheets present: ${COMPONENTS.join(", ")}.`); diff --git a/scripts/check-pack.mjs b/scripts/check-pack.mjs index 250cd5d..e1e04ba 100644 --- a/scripts/check-pack.mjs +++ b/scripts/check-pack.mjs @@ -7,11 +7,22 @@ * This packs the real tarball, asserts nothing unexpected is inside it, and * asserts every path the exports map advertises actually resolves to a packed * file, so a broken subpath is caught here rather than by a consumer. + * + * It also asserts the converse, which is how 0.1.0-alpha.0 shipped every + * component unstyled: each of the 20 emitted stylesheets was packed, imported + * by nothing, and named by no exports entry, so a consumer had no supported + * way to load it. Checking that advertised paths resolve says nothing about + * files that arrive advertised by no one. */ import { execFileSync } from "node:child_process"; import { readFile } from "node:fs/promises"; import { fileURLToPath } from "node:url"; import { dirname, resolve } from "node:path"; +import { + dirname as posixDirname, + join as posixJoin, + normalize as posixNormalize, +} from "node:path/posix"; const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); @@ -56,17 +67,13 @@ for (const file of packed) { const pkg = JSON.parse(await readFile(resolve(root, "package.json"), "utf8")); const packedSet = new Set(packed); -function assertResolves(target, subpath) { +/** Turn an exports target into a predicate over packed file paths. */ +function targetMatcher(target) { const clean = target.replace(/^\.\//, ""); - if (!clean.includes("*")) { - if (!packedSet.has(clean)) { - failures.push(`exports["${subpath}"] points at "${clean}", which is not packed`); - } - return; - } + if (!clean.includes("*")) return (file) => file === clean; - const matcher = new RegExp( + const pattern = new RegExp( "^" + clean .split("*") @@ -74,20 +81,67 @@ function assertResolves(target, subpath) { .join(".+") + "$", ); - if (!packed.some((f) => matcher.test(f))) { + return (file) => pattern.test(file); +} + +function assertResolves(target, subpath) { + const clean = target.replace(/^\.\//, ""); + const matches = targetMatcher(target); + + if (!clean.includes("*")) { + if (!packedSet.has(clean)) { + failures.push(`exports["${subpath}"] points at "${clean}", which is not packed`); + } + return; + } + + if (!packed.some(matches)) { failures.push(`exports["${subpath}"] pattern "${clean}" matches nothing packed`); } } +const exportedMatchers = []; + for (const [subpath, target] of Object.entries(pkg.exports ?? {})) { if (subpath === "./package.json") continue; if (typeof target === "string") { assertResolves(target, subpath); + exportedMatchers.push(targetMatcher(target)); } else { - for (const value of Object.values(target)) assertResolves(value, subpath); + for (const value of Object.values(target)) { + assertResolves(value, subpath); + exportedMatchers.push(targetMatcher(value)); + } } } +/** + * Every packed stylesheet has to be loadable. A component's CSS is loadable + * because the chunk that owns it imports it (see `linkComponentStyles` in + * vite.config.ts); a token or theme stylesheet is loadable because the exports + * map names it and a consumer imports it directly. A stylesheet that is + * neither reaches nobody, and the build stays green while every rule in it + * goes missing at the consumer. + */ +const importedStylesheets = new Set(); + +for (const file of packed.filter((f) => f.endsWith(".js"))) { + const code = await readFile(resolve(root, file), "utf8"); + for (const [, specifier] of code.matchAll(/\bimport\s*["']([^"']+\.css)["']/g)) { + if (!specifier.startsWith(".")) continue; // resolved at the consumer + importedStylesheets.add(posixNormalize(posixJoin(posixDirname(file), specifier))); + } +} + +for (const stylesheet of packed.filter((f) => f.endsWith(".css"))) { + if (importedStylesheets.has(stylesheet)) continue; + if (exportedMatchers.some((matches) => matches(stylesheet))) continue; + failures.push( + `"${stylesheet}" is packed but unreachable: no packed module imports it and no ` + + `exports entry names it, so a consumer cannot load its rules`, + ); +} + if (failures.length > 0) { console.error(`Packed artifact check failed (${failures.length}):\n`); for (const f of failures) console.error(` ${f}`); diff --git a/vite.config.ts b/vite.config.ts index fd366e3..ce11a20 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -1,7 +1,7 @@ import { cp, mkdir } from "node:fs/promises"; import { existsSync } from "node:fs"; import { fileURLToPath } from "node:url"; -import { dirname, isAbsolute, resolve } from "node:path"; +import { dirname, isAbsolute, posix, relative, resolve, sep } from "node:path"; import react from "@vitejs/plugin-react"; import type { Plugin } from "vite"; @@ -49,11 +49,100 @@ function copyStyles(): Plugin { }; } +/** Strip a query suffix and express an id relative to the repository root. */ +function sourceKey(id: string): string { + const path = id.split("?")[0] ?? id; + const absolute = isAbsolute(path) ? path : resolve(root, path); + return relative(root, absolute).split(sep).join("/"); +} + +/** + * Re-attach each emitted stylesheet to the chunk whose module imported it. + * + * In library mode Rollup strips `import "./Button.css"` out of the chunk and + * leaves an "empty css" marker comment, because an application build would + * have injected a link tag instead. Nothing puts the import back, so a published + * package can carry a component's stylesheet as an emitted asset that no code + * path reaches. That is what 0.1.0-alpha.0 shipped: 20 stylesheets under + * `dist/assets/`, none imported by any chunk, and no exports entry pointing at + * them either, so a consumer rendered every component unstyled with no + * supported way to fix it. Both the package build and the install fixture were + * green throughout, since neither one looked at whether an emitted asset was + * reachable. + * + * Restoring the import here is what makes README's "component CSS travels with + * the component" true, and keeps `styles/base.css` the tokens-only entry point + * it is documented to be rather than a second thing to remember. Imports are + * appended rather than prepended: ES modules hoist them, so execution order is + * unchanged while every existing line keeps its position and the sourcemap + * emitted moments earlier stays accurate. + */ +function linkComponentStyles(): Plugin { + return { + name: "ui-common-link-component-styles", + apply: "build", + enforce: "post", + generateBundle(_options, bundle) { + const emittedBySource = new Map(); + + for (const file of Object.values(bundle)) { + if (file.type !== "asset" || !file.fileName.endsWith(".css")) continue; + for (const original of file.originalFileNames ?? []) { + emittedBySource.set(sourceKey(original), file.fileName); + } + } + + for (const file of Object.values(bundle)) { + if (file.type !== "chunk") continue; + + // The stripped import is gone from `moduleIds`, but the edge that + // produced it survives in the module graph, so the graph is what this + // reads. Pairing `Button.css` with `Button.tsx` by filename would + // agree on every component the package has today and quietly stop + // agreeing on the first one that breaks the convention. + const specifiers: string[] = []; + for (const id of file.moduleIds) { + for (const imported of this.getModuleInfo(id)?.importedIds ?? []) { + if (!imported.endsWith(".css")) continue; + + const key = sourceKey(imported); + // A stylesheet from a dependency stays a bare specifier that the + // consumer resolves; only this package's own files are re-linked. + if (key.startsWith("..")) continue; + + const emitted = emittedBySource.get(key); + if (!emitted) { + this.error( + `${file.fileName} imports "${key}", which was not emitted as an asset. ` + + `Its rules would ship with nothing able to reach them. ` + + `Emitted stylesheets: ${[...emittedBySource.keys()].join(", ")}`, + ); + } + + const relativeToChunk = posix.relative( + posix.dirname(file.fileName), + emitted, + ); + const specifier = relativeToChunk.startsWith(".") + ? relativeToChunk + : `./${relativeToChunk}`; + if (!specifiers.includes(specifier)) specifiers.push(specifier); + } + } + + if (specifiers.length === 0) continue; + file.code += `\n${specifiers.map((s) => `import "${s}";`).join("\n")}\n`; + } + }, + }; +} + export default defineConfig({ plugins: [ react(), dts({ include: ["src"], exclude: ["src/**/*.test.*", "src/test/**"] }), copyStyles(), + linkComponentStyles(), ], build: { target: "es2022",