⚠️ [WB-2324.2] Phase 0.3 — Type generation + IDE autocomplete for CSS Modules#3063
Conversation
🦋 Changeset detectedLatest commit: c06d53d The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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 |
|
Size Change: 0 B Total Size: 129 kB ℹ️ View Unchanged
|
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (6cd221f) and published all packages with changesets to npm. You can install the packages in ./dev/tools/deploy_wonder_blocks.js --tag="PR3063"Packages can also be installed manually by running: pnpm add @khanacademy/wonder-blocks-<package-name>@PR3063 |
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-kccwzddppx.chromatic.com/ Chromatic results:
|
| "alex": "alex packages/", | ||
| "dev": "pnpm build && turbo watch build:css", | ||
| "build": "rollup -c ./build-settings/rollup.config.mjs && pnpm -r build && turbo build:css", | ||
| "prebuild": "pnpm gen:css-types", |
There was a problem hiding this comment.
typed-css-modules + typescript-plugin-css-modules devDeps and wires gen:css-types into both prebuild and pretypecheck (line 43). Hooks run for every pnpm build and every pnpm typecheck, so a regression in build-settings/gen-css-types.cjs would break CI. cc: @Khan/wonder-blocks
| // Provides live IDE autocomplete and "Go to Definition" for | ||
| // `*.module.css` imports. Build-time type safety still comes from | ||
| // the generated `*.module.css.d.ts` files (see `gen:css-types`). | ||
| "plugins": [{"name": "typescript-plugin-css-modules"}] |
There was a problem hiding this comment.
typescript-plugin-css-modules TS language-service plugin. Doesn't affect tsc output (plugins only influence the language service), but everyone using a TS-aware editor on this repo will start seeing the plugin's behavior — live autocomplete + Go-to-Definition for styles.<className>. cc: @Khan/wonder-blocks
There was a problem hiding this comment.
Would it make sense to include instructions for setting up this ts plugin in the getting started docs? https://khan.github.io/wonder-blocks/?path=/docs/get-started--docs
Or maybe somewhere else, since it isn't specific to WB?
| @@ -0,0 +1,48 @@ | |||
| // Generate `*.module.css.d.ts` files next to every `*.module.css` source. | |||
There was a problem hiding this comment.
🔷 3 of 3: Root-level Node script that globs packages/**/*.module.css + __docs__/**/*.module.css (ignoring node_modules/dist) and invokes typed-css-modules to emit colocated .d.ts declarations. Camel-casing left disabled (camelCase: false) until 0.4 picks the project-wide naming convention — the spike's class names already happen to be camelCase, so this is a no-op for now. cc: @Khan/wonder-blocks
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## WB-2324 #3063 +/- ##
===============================
===============================
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
jandrade
changed the title
Adds CSS-side hygiene to the migration. `pnpm lint:css` is now an explicit CI step alongside ESLint and Typecheck. Token-required properties (color / spacing / typography) raise a warning when authored as raw values — stays at warn level during the migration and tightens to error once Phase 5 finishes. Builds on top of WB-2324.3 (PR #3063); no runtime behavior changes. Reviewers: #frontend-infra-web Issue: WB-2324 ## Test plan: 1. `pnpm lint:css` — passes (0 errors, 1 pre-existing warning on `__docs__/wonder-blocks-typography/styles.css` for a hardcoded `color: red`). 2. Add a bogus at-rule body to `spike.module.css` (e.g. `@whatever-bogus-rule { color: red; }`): `pnpm lint:css` fails with `Unknown at-rule "@whatever-bogus-rule"` and warns about the raw color. 3. Revert → clean. 4. Watch CI on this branch: the new `Stylelint` step runs after `Eslint` and before `Typecheck` in node-ci-lint.yml. ## Review plan: Please review these risky changes 1. 🚨 `.github/workflows/node-ci-lint.yml`: adds a new `Stylelint` job step (`pnpm lint:css`) between `Eslint` and `Typecheck`. Runs on every PR; a regression in the stylelint config or new false-positive rule could block unrelated work. `if: always()` so it doesn't short-circuit on an upstream failure. 2.⚠️ `.stylelintrc.cjs` (new): extends `stylelint-config-standard` + `stylelint-config-css-modules`. Allow-lists `@apply`, `@mixin`, `@define-mixin`, `@include`, `@layer` so the PostCSS pipeline's at-rules pass. Disables `custom-property-pattern` (WB tokens use `--wb-camelCase_underscore` naming on purpose). Activates `scale-unlimited/declaration-strict-value` at *warning* level for color / background / border-color / padding / margin / font-size / font-family. 3.⚠️ `package.json`: adds 4 external devDeps — `stylelint`, `stylelint-config-standard`, `stylelint-config-css-modules`, `stylelint-declaration-strict-value` — plus the `lint:css` script. New script becomes part of standard dev / CI workflow. ### Common patterns: **1 File — declarative at-rule allow-list paired with the PostCSS pipeline:** ```js // .stylelintrc.cjs — must match plugin order in postcss.config.cjs "at-rule-no-unknown": [ true, { ignoreAtRules: [ "apply", "mixin", "define-mixin", "include", "layer", ], }, ], ``` **1 File — CI job step that mirrors the existing Eslint / Typecheck shape:** ```yaml # .github/workflows/node-ci-lint.yml - name: Stylelint if: always() run: pnpm lint:css ``` 🤖 Built using Claude Code #ai-generated
|
@claude review once |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — tooling-only PR, the inline nit about stale *.d.ts cleanup is non-blocking.
Extended reasoning...
Overview
This PR is the Phase 0.3 tooling for the CSS Modules migration. It adds two devDeps (typed-css-modules, typescript-plugin-css-modules), a small Node script at build-settings/gen-css-types.cjs that globs *.module.css and emits colocated *.module.css.d.ts files, wires that script into prebuild and pretypecheck in package.json, registers the TS language-service plugin in tsconfig.json, gitignores/eslintignores the generated declarations, and recommends a VS Code extension. No runtime or Rollup behavior changes.
Security risks
None apparent. No auth, crypto, network, or user-input surfaces are touched. The new devDeps run in build/typecheck context only and the generated .d.ts files are gitignored.
Level of scrutiny
Low-to-moderate. This is build tooling, not production runtime code, and the author has already self-annotated the three risky changes (package.json, tsconfig.json, gen-css-types.cjs) with clear reasoning. The bug hunter surfaced only one nit-level issue (stale *.d.ts files when CSS sources are renamed/deleted), which is bounded to local working trees — CI starts from a fresh checkout and is unaffected. It is a reasonable refinement to defer to Phase 0.4 as the migration surface grows.
Other factors
- Self-contained, easily reversible change.
- The CSS Modules spike currently has exactly one
*.module.cssfile, so the blast radius for any regression in the generator is minimal today. - Reviewer requested approval explicitly (
@claude review once) and no prior review from me exists on this PR. - Generated
.d.tsfiles are correctly excluded from both.gitignoreand.eslintignore.
| for (const file of files) { | ||
| const content = await creator.create(file); | ||
| await content.writeFile(); | ||
| } |
There was a problem hiding this comment.
🟡 nit: gen-css-types.cjs regenerates a .d.ts for each *.module.css it finds but never deletes orphaned *.module.css.d.ts files whose source has been renamed or removed. The stale declarations let import styles from './removed.module.css' keep type-checking locally (CI is unaffected since the files are gitignored). Consider unlinking *.module.css.d.ts files without a matching source at the start of each run.
Extended reasoning...
What the bug is
build-settings/gen-css-types.cjs globs every packages/**/*.module.css and __docs__/**/*.module.css source and writes a colocated <name>.module.css.d.ts via typed-css-modules. It only ever writes; it never enumerates existing .d.ts files to prune ones whose source has disappeared. So after a developer renames foo.module.css → bar.module.css (or deletes the CSS source entirely), foo.module.css.d.ts lingers in the working tree.
Why it matters (and the one real consequence)
Because *.module.css.d.ts acts as the ambient type declaration for *.module.css, TypeScript will still resolve import styles from './foo.module.css' against the orphan declaration rather than failing with Cannot find module. pnpm typecheck therefore reports green on a local checkout that has stale .d.ts files, even though a fresh checkout (CI) would correctly fail since the .d.ts files are gitignored. The realistic failure mode is: a dev renames/deletes a CSS module file, forgets to update one importer, local typecheck passes, and the diff lands in a PR where CI then fails — or worse, the importer is in code that doesn't get exercised until the build step.
Scope is narrow (hence nit)
The blast radius is limited and worth being upfront about:
- CI / fresh checkouts are unaffected —
.d.tsfiles are gitignored and regenerated byprebuild/pretypecheck. - Class-name renames within the same file are not affected —
typed-css-modules'writeFile()overwrites the colocated.d.tsrather than merging, so removed classes do disappear on regeneration. pnpm build(Rollup) loads the real CSS file and would fail with a missing-module error if the source is gone.- The IDE side uses
typescript-plugin-css-modules, which reads CSS files live, so editors would also surface the missing source.
So the only window is pnpm typecheck against a stale local working tree, between the rename/delete and the next git clean / fresh checkout. Today only one CSS Module file exists (__docs__/css-modules-spike/spike.module.css), so the chance of hitting it in Phase 0.3 is low — but the surface grows as Phase 0.4+ migrates more files.
Step-by-step proof
pnpm gen:css-typesruns in a clean tree —__docs__/css-modules-spike/spike.module.css.d.tsis written declaringpillandroot.- Developer renames the file:
git mv spike.module.css renamed.module.cssand updates theimportinspike.tsxto'./renamed.module.css'. They forget to update one other hypothetical importer (call itother.tsx) that still hasimport styles from './spike.module.css'. pnpm typecheckinvokespretypecheck→gen:css-types. The script globs sources, findsrenamed.module.css, writesrenamed.module.css.d.ts. It never inspectsspike.module.css.d.ts, which is still on disk from step 1.tscruns.other.tsx's import of./spike.module.cssresolves against the stalespike.module.css.d.ts, andstyles.pillstill type-checks. Typecheck passes locally.- The PR lands on CI. CI starts from a fresh checkout (no orphan
.d.ts), regenerates onlyrenamed.module.css.d.ts, and nowother.tsx's import fails withCannot find module './spike.module.css'. Red build, surprised author.
How to fix
At the start of main(), glob **/*.module.css.d.ts (same roots, same ignores) and fs.unlink any whose corresponding *.module.css source no longer exists — or, equivalently, walk the source list to build a set of expected output paths and remove every *.module.css.d.ts not in the set. Either approach keeps prebuild / pretypecheck idempotent without affecting CI behavior.
Addressing the refutation
The refuter is correct that the original synthesis description over-claims — consequences #2 (class-name renames within a file) and #3 (new-contributor artifacts) are not real, for the reasons above. This comment is narrowed to the one real consequence (rename/delete of a CSS source). The refuter's other point — that this is a known limitation of typed-css-modules tooling and could be deferred to Phase 0.4 — is fair; that's why this is filed as nit rather than blocking. A ~10-line cleanup pass at the top of the script removes the local/CI divergence cheaply, but it's a reasonable refinement to land later.
There was a problem hiding this comment.
Nice! Confirmed type checking catches if I try using a style that doesn't exist:
In terms of DX, do you know if there's a way to make it so if I cmd + click on the import file name (ie. ./spike.module.css), it'll take me to the css file, rather than the type declaration file? Before (in the WB-2324 branch), it gave me an option to choose the .css file. After, it'll take me to the spike.module.css.d.ts file directly.
| Before | After |
|---|---|
 |
 |
It would be nice if cmd + click on the styles variable took the user to the .css file too!
Update: Nvm! I had to install the recommended extensions and reload 😄 cmd+click on the file name gives me the option to go to the CSS file. cmd+click on the classname (the root part of styles.root), takes me to the css class definition, though cmd+click on the styles part takes me to the assets.d.ts file instead!
| // Provides live IDE autocomplete and "Go to Definition" for | ||
| // `*.module.css` imports. Build-time type safety still comes from | ||
| // the generated `*.module.css.d.ts` files (see `gen:css-types`). | ||
| "plugins": [{"name": "typescript-plugin-css-modules"}] |
There was a problem hiding this comment.
Would it make sense to include instructions for setting up this ts plugin in the getting started docs? https://khan.github.io/wonder-blocks/?path=/docs/get-started--docs
Or maybe somewhere else, since it isn't specific to WB?
|
Another DX thing I noticed, what's the best way to identify what uses a With React components, I'm able to use the "Find File References" option. This option isn't available for the
|
Adds CSS-side hygiene to the migration. `pnpm lint:css` is now an explicit CI step alongside ESLint and Typecheck. Token-required properties (color / spacing / typography) raise a warning when authored as raw values — stays at warn level during the migration and tightens to error once Phase 5 finishes. Builds on top of WB-2324.3 (PR #3063); no runtime behavior changes. Reviewers: #frontend-infra-web Issue: WB-2324 ## Test plan: 1. `pnpm lint:css` — passes (0 errors, 1 pre-existing warning on `__docs__/wonder-blocks-typography/styles.css` for a hardcoded `color: red`). 2. Add a bogus at-rule body to `spike.module.css` (e.g. `@whatever-bogus-rule { color: red; }`): `pnpm lint:css` fails with `Unknown at-rule "@whatever-bogus-rule"` and warns about the raw color. 3. Revert → clean. 4. Watch CI on this branch: the new `Stylelint` step runs after `Eslint` and before `Typecheck` in node-ci-lint.yml. ## Review plan: Please review these risky changes 1. 🚨 `.github/workflows/node-ci-lint.yml`: adds a new `Stylelint` job step (`pnpm lint:css`) between `Eslint` and `Typecheck`. Runs on every PR; a regression in the stylelint config or new false-positive rule could block unrelated work. `if: always()` so it doesn't short-circuit on an upstream failure. 2.⚠️ `.stylelintrc.cjs` (new): extends `stylelint-config-standard` + `stylelint-config-css-modules`. Allow-lists `@apply`, `@mixin`, `@define-mixin`, `@include`, `@layer` so the PostCSS pipeline's at-rules pass. Disables `custom-property-pattern` (WB tokens use `--wb-camelCase_underscore` naming on purpose). Activates `scale-unlimited/declaration-strict-value` at *warning* level for color / background / border-color / padding / margin / font-size / font-family. 3.⚠️ `package.json`: adds 4 external devDeps — `stylelint`, `stylelint-config-standard`, `stylelint-config-css-modules`, `stylelint-declaration-strict-value` — plus the `lint:css` script. New script becomes part of standard dev / CI workflow. ### Common patterns: **1 File — declarative at-rule allow-list paired with the PostCSS pipeline:** ```js // .stylelintrc.cjs — must match plugin order in postcss.config.cjs "at-rule-no-unknown": [ true, { ignoreAtRules: [ "apply", "mixin", "define-mixin", "include", "layer", ], }, ], ``` **1 File — CI job step that mirrors the existing Eslint / Typecheck shape:** ```yaml # .github/workflows/node-ci-lint.yml - name: Stylelint if: always() run: pnpm lint:css ``` 🤖 Built using Claude Code #ai-generated
jeresig
left a comment
jeresig
left a comment
There was a problem hiding this comment.
I think I'm not understanding the changes in this PR. It seems like typescript-plugin-css-modules allows us to dynamically compute the types in Typescript - but then how are the generated type files used? They aren't checked in... are this distributed as part of the package? I thought we weren't exporting any CSS modules directly?
Good questions @jeresig! This is more for internal WB consumption as we are not planning to distribute these typedefs as part of the package.
My plan is to implement something similar in consumer repos in a way that folks can also use these tools to enforce that we are using valid css modules. I'm going to try to see if we can use a tool in |
jeresig
left a comment
jeresig
left a comment
There was a problem hiding this comment.
@jandrade Gotcha, ok that's helpful! So I think it may be worth confirming that in frontend when doing a build using Rspack that it doesn't error out anyway, as we may already have something that's able to catch failures. But it's possible that this would be "too late" and we'd want to catch it during typechecking anyway, as you noted.
Adds CSS-side hygiene to the migration. `pnpm lint:css` is now an explicit CI step alongside ESLint and Typecheck. Token-required properties (color / spacing / typography) raise a warning when authored as raw values — stays at warn level during the migration and tightens to error once Phase 5 finishes. Builds on top of WB-2324.3 (PR #3063); no runtime behavior changes. Reviewers: #frontend-infra-web Issue: WB-2324 ## Test plan: 1. `pnpm lint:css` — passes (0 errors, 1 pre-existing warning on `__docs__/wonder-blocks-typography/styles.css` for a hardcoded `color: red`). 2. Add a bogus at-rule body to `spike.module.css` (e.g. `@whatever-bogus-rule { color: red; }`): `pnpm lint:css` fails with `Unknown at-rule "@whatever-bogus-rule"` and warns about the raw color. 3. Revert → clean. 4. Watch CI on this branch: the new `Stylelint` step runs after `Eslint` and before `Typecheck` in node-ci-lint.yml. ## Review plan: Please review these risky changes 1. 🚨 `.github/workflows/node-ci-lint.yml`: adds a new `Stylelint` job step (`pnpm lint:css`) between `Eslint` and `Typecheck`. Runs on every PR; a regression in the stylelint config or new false-positive rule could block unrelated work. `if: always()` so it doesn't short-circuit on an upstream failure. 2.⚠️ `.stylelintrc.cjs` (new): extends `stylelint-config-standard` + `stylelint-config-css-modules`. Allow-lists `@apply`, `@mixin`, `@define-mixin`, `@include`, `@layer` so the PostCSS pipeline's at-rules pass. Disables `custom-property-pattern` (WB tokens use `--wb-camelCase_underscore` naming on purpose). Activates `scale-unlimited/declaration-strict-value` at *warning* level for color / background / border-color / padding / margin / font-size / font-family. 3.⚠️ `package.json`: adds 4 external devDeps — `stylelint`, `stylelint-config-standard`, `stylelint-config-css-modules`, `stylelint-declaration-strict-value` — plus the `lint:css` script. New script becomes part of standard dev / CI workflow. ### Common patterns: **1 File — declarative at-rule allow-list paired with the PostCSS pipeline:** ```js // .stylelintrc.cjs — must match plugin order in postcss.config.cjs "at-rule-no-unknown": [ true, { ignoreAtRules: [ "apply", "mixin", "define-mixin", "include", "layer", ], }, ], ``` **1 File — CI job step that mirrors the existing Eslint / Typecheck shape:** ```yaml # .github/workflows/node-ci-lint.yml - name: Stylelint if: always() run: pnpm lint:css ``` 🤖 Built using Claude Code #ai-generated
…odules Adds the static-typing half of the CSS Modules migration tooling. `tsc --noEmit` now catches `styles.<missingClassName>` references, and VS Code / WebStorm get live autocomplete + Go-to-Definition on `*.module.css` imports. Builds on top of `WB-2324` (PR #3059); no Rollup or runtime behavior changes. Reviewers: #frontend-infra-web Issue: WB-2324 ## Test plan: 1. `pnpm typecheck` — passes; `prebuild`/`pretypecheck` regenerates `.d.ts` files first. 2. Introduce `styles.tpyo` in `__docs__/css-modules-spike/spike.tsx` → `pnpm typecheck` fails with `Property 'tpyo' does not exist on type '{ readonly pill: string; readonly root: string; }'. Did you mean 'root'?`. 3. Revert the typo → `pnpm typecheck` passes again. 4. `pnpm build` — runs `gen:css-types` first via the `prebuild` hook, then Rollup; build succeeds. 5. Open `__docs__/css-modules-spike/spike.tsx` in VS Code with the recommended extensions installed: `styles.` autocomplete shows `pill` and `root`; Go-to-Definition jumps to the matching CSS rule. ## Review plan: Please review these risky changes 1.⚠️ `package.json`: adds `typed-css-modules` + `typescript-plugin-css-modules` devDeps and wires `gen:css-types` into both `prebuild` and `pretypecheck`. Hooks run for every `pnpm build` and every `pnpm typecheck`, so a regression in `gen-css-types.cjs` would break CI. 2.⚠️ `tsconfig.json`: adds the `typescript-plugin-css-modules` TS language-service plugin. Doesn't affect `tsc` output (plugins only influence the language service), but everyone using a TS-aware editor on this repo will start seeing the plugin's behavior. 3. 🔷 `build-settings/gen-css-types.cjs` (new): root-level script that globs `packages/**/*.module.css` + `__docs__/**/*.module.css` (ignoring `node_modules`/`dist`) and invokes `typed-css-modules`. Camel-casing left disabled until 0.4 picks the project-wide naming convention. ### Common patterns: **2 Files — pre-script hooks ensuring `*.module.css.d.ts` exists before tsc / Rollup run:** ```jsonc // package.json + "prebuild": "pnpm gen:css-types", + "gen:css-types": "node ./build-settings/gen-css-types.cjs", ... + "pretypecheck": "pnpm gen:css-types", ``` **1 File — gitignore the generated declarations (regenerated on every build):** ``` # Generated by `pnpm gen:css-types` (typed-css-modules) via the `prebuild` / # `pretypecheck` hooks. Never checked in — fresh checkouts regenerate them. **/*.module.css.d.ts ``` 🤖 Built using Claude Code #ai-generated
Mirrors the .gitignore entry — these are codegen artifacts from `pnpm gen:css-types` and don't need to pass prettier or eslint checks. Without this, `pnpm lint` flags the generated declarations for inconsistent quoting + trailing newline. 🤖 Built using Claude Code #ai-generated
Adds CSS-side hygiene to the migration. `pnpm lint:css` is now an explicit CI step alongside ESLint and Typecheck. Token-required properties (color / spacing / typography) raise a warning when authored as raw values — stays at warn level during the migration and tightens to error once Phase 5 finishes. Builds on top of WB-2324.3 (PR #3063); no runtime behavior changes. Reviewers: #frontend-infra-web Issue: WB-2324 ## Test plan: 1. `pnpm lint:css` — passes (0 errors, 1 pre-existing warning on `__docs__/wonder-blocks-typography/styles.css` for a hardcoded `color: red`). 2. Add a bogus at-rule body to `spike.module.css` (e.g. `@whatever-bogus-rule { color: red; }`): `pnpm lint:css` fails with `Unknown at-rule "@whatever-bogus-rule"` and warns about the raw color. 3. Revert → clean. 4. Watch CI on this branch: the new `Stylelint` step runs after `Eslint` and before `Typecheck` in node-ci-lint.yml. ## Review plan: Please review these risky changes 1. 🚨 `.github/workflows/node-ci-lint.yml`: adds a new `Stylelint` job step (`pnpm lint:css`) between `Eslint` and `Typecheck`. Runs on every PR; a regression in the stylelint config or new false-positive rule could block unrelated work. `if: always()` so it doesn't short-circuit on an upstream failure. 2.⚠️ `.stylelintrc.cjs` (new): extends `stylelint-config-standard` + `stylelint-config-css-modules`. Allow-lists `@apply`, `@mixin`, `@define-mixin`, `@include`, `@layer` so the PostCSS pipeline's at-rules pass. Disables `custom-property-pattern` (WB tokens use `--wb-camelCase_underscore` naming on purpose). Activates `scale-unlimited/declaration-strict-value` at *warning* level for color / background / border-color / padding / margin / font-size / font-family. 3.⚠️ `package.json`: adds 4 external devDeps — `stylelint`, `stylelint-config-standard`, `stylelint-config-css-modules`, `stylelint-declaration-strict-value` — plus the `lint:css` script. New script becomes part of standard dev / CI workflow. ### Common patterns: **1 File — declarative at-rule allow-list paired with the PostCSS pipeline:** ```js // .stylelintrc.cjs — must match plugin order in postcss.config.cjs "at-rule-no-unknown": [ true, { ignoreAtRules: [ "apply", "mixin", "define-mixin", "include", "layer", ], }, ], ``` **1 File — CI job step that mirrors the existing Eslint / Typecheck shape:** ```yaml # .github/workflows/node-ci-lint.yml - name: Stylelint if: always() run: pnpm lint:css ``` 🤖 Built using Claude Code #ai-generated
Adds CSS-side hygiene to the migration. `pnpm lint:css` is now an explicit CI step alongside ESLint and Typecheck. Token-required properties (color / spacing / typography) raise a warning when authored as raw values — stays at warn level during the migration and tightens to error once Phase 5 finishes. Builds on top of WB-2324.3 (PR #3063); no runtime behavior changes. Reviewers: #frontend-infra-web Issue: WB-2324 ## Test plan: 1. `pnpm lint:css` — passes (0 errors, 1 pre-existing warning on `__docs__/wonder-blocks-typography/styles.css` for a hardcoded `color: red`). 2. Add a bogus at-rule body to `spike.module.css` (e.g. `@whatever-bogus-rule { color: red; }`): `pnpm lint:css` fails with `Unknown at-rule "@whatever-bogus-rule"` and warns about the raw color. 3. Revert → clean. 4. Watch CI on this branch: the new `Stylelint` step runs after `Eslint` and before `Typecheck` in node-ci-lint.yml. ## Review plan: Please review these risky changes 1. 🚨 `.github/workflows/node-ci-lint.yml`: adds a new `Stylelint` job step (`pnpm lint:css`) between `Eslint` and `Typecheck`. Runs on every PR; a regression in the stylelint config or new false-positive rule could block unrelated work. `if: always()` so it doesn't short-circuit on an upstream failure. 2.⚠️ `.stylelintrc.cjs` (new): extends `stylelint-config-standard` + `stylelint-config-css-modules`. Allow-lists `@apply`, `@mixin`, `@define-mixin`, `@include`, `@layer` so the PostCSS pipeline's at-rules pass. Disables `custom-property-pattern` (WB tokens use `--wb-camelCase_underscore` naming on purpose). Activates `scale-unlimited/declaration-strict-value` at *warning* level for color / background / border-color / padding / margin / font-size / font-family. 3.⚠️ `package.json`: adds 4 external devDeps — `stylelint`, `stylelint-config-standard`, `stylelint-config-css-modules`, `stylelint-declaration-strict-value` — plus the `lint:css` script. New script becomes part of standard dev / CI workflow. ### Common patterns: **1 File — declarative at-rule allow-list paired with the PostCSS pipeline:** ```js // .stylelintrc.cjs — must match plugin order in postcss.config.cjs "at-rule-no-unknown": [ true, { ignoreAtRules: [ "apply", "mixin", "define-mixin", "include", "layer", ], }, ], ``` **1 File — CI job step that mirrors the existing Eslint / Typecheck shape:** ```yaml # .github/workflows/node-ci-lint.yml - name: Stylelint if: always() run: pnpm lint:css ``` 🤖 Built using Claude Code #ai-generated
…ng (#3069) ## Summary: Adds CSS-side hygiene to the migration. `pnpm lint:css` is now an explicit CI step alongside ESLint and Typecheck. Highlights: - **At-rule allow-list** lets `@apply`, `@mixin`, `@define-mixin`, `@include`, `@layer` pass through (matches `postcss.config.cjs`). - **`selector-class-pattern`** enforces camelCase — `.root`, `.iconWrapper`, etc. - **Token-required properties** (color / spacing / typography) raise a *warning* when authored as raw values; tightens to error once Phase 5 finishes. - **`csstools/value-no-unknown-custom-properties`** rejects any `var(--foo)` that doesn't resolve to a token published by `@khanacademy/wonder-blocks-tokens` (reads `packages/wonder-blocks-tokens/dist/css/index.css`). - **`prelint:css`** runs `turbo build:css` so the merged token CSS is fresh before stylelint reads it (CI already builds first; locally turbo's cache makes warm runs near-instant). Builds on top of WB-2324.3 (PR #3063); no runtime behavior changes. Reviewers: jandrade, claude[bot], beaesguerra, jeresig Issue: WB-2324 ## Test plan: 1. `pnpm lint:css` — passes (0 errors, 1 pre-existing warning on `__docs__/wonder-blocks-typography/styles.css` for a hardcoded `color: red`). 2. Add a bogus at-rule body to `spike.module.css` (e.g. `@whatever-bogus-rule { color: red; }`): `pnpm lint:css` fails with `Unknown at-rule "@whatever-bogus-rule"` and warns about the raw color. 3. Change a `var(--wb-sizing-size_080)` reference to `var(--wb-totally-bogus-token)` in `spike.module.css`: `pnpm lint:css` fails with `Unexpected custom property "--wb-totally-bogus-token"`. 4. Rename a CSS Modules class to kebab-case (e.g. `.pill-badge`): `pnpm lint:css` fails with `Expected ".pill-badge" to match pattern "^[a-z]+([A-Z][a-z0-9]+)*$"`. 5. Revert all of the above → clean. 6. Watch CI on this branch: the new `Stylelint` step runs after `Eslint` and before `Typecheck` in `node-ci-lint.yml`. ## Review plan: Please review these risky changes — each links to its inline comment: 1. 🚨 [`.github/workflows/node-ci-lint.yml`](#3069 (comment)) 2.⚠️ [`.stylelintrc.cjs`](#3069 (comment)) 3.⚠️ [`package.json`](#3069 (comment)) ### Common patterns: **1 File — declarative at-rule allow-list paired with the PostCSS pipeline:** ```js // .stylelintrc.cjs — must match plugin order in postcss.config.cjs "at-rule-no-unknown": [ true, { ignoreAtRules: [ "apply", "mixin", "define-mixin", "include", "layer", ], }, ], ``` **1 File — token-anchored `var()` validation:** ```js // .stylelintrc.cjs "csstools/value-no-unknown-custom-properties": [ true, { importFrom: [ "./packages/wonder-blocks-tokens/dist/css/index.css", ], }, ], ``` **1 File — CI job step that mirrors the existing Eslint / Typecheck shape:** ```yaml # .github/workflows/node-ci-lint.yml - name: Stylelint if: always() run: pnpm lint:css ``` 🤖 Built using Claude Code #ai-generated Author: jandrade Required Reviewers: Approved By: jeresig, beaesguerra Checks: ✅ 13 checks were successful, ⏭️ 3 checks have been skipped Pull Request URL: #3069
3 participants
Summary:
Adds the static-typing half of the CSS Modules migration tooling.
tsc --noEmitnow catchesstyles.<missingClassName>references, and VS Code / WebStorm get live autocomplete + Go-to-Definition on*.module.cssimports.Builds on top of
WB-2324(PR #3059); no Rollup or runtime behavior changes.Reviewers: #frontend-infra-web
Issue: WB-2324
Test plan:
pnpm typecheck— passes;prebuild/pretypecheckregenerates.d.tsfiles first.styles.tpyoin__docs__/css-modules-spike/spike.tsx→pnpm typecheckfails withProperty 'tpyo' does not exist on type '{ readonly pill: string; readonly root: string; }'. Did you mean 'root'?.pnpm typecheckpasses again.pnpm build— runsgen:css-typesfirst via theprebuildhook, then Rollup; build succeeds.__docs__/css-modules-spike/spike.tsxin VS Code with the recommended extensions installed:styles.autocomplete showspillandroot; Go-to-Definition jumps to the matching CSS rule.Review plan:
Please review these risky changes — each links to its inline comment:
package.jsontsconfig.jsonbuild-settings/gen-css-types.cjsCommon patterns:
2 Files — pre-script hooks ensuring
*.module.css.d.tsexists before tsc / Rollup run:1 File — gitignore the generated declarations (regenerated on every build):
🤖 Built using Claude Code #ai-generated