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",