feat(css): complete Tailwind v4 generation flow alignment (fixes #648)#650
feat(css): complete Tailwind v4 generation flow alignment (fixes #648)#650diyor28 wants to merge 2 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
diyor28
left a comment
There was a problem hiding this comment.
🤖 Codex PR review — feat(css): complete Tailwind v4 generation flow alignment (DRAFT)
Read-only review. Reviewed fully despite DRAFT status (CI is skipped, so there's currently no automated signal on correctness). 4 inline comments above. This is SDK-level infra consumed by every downstream app, so the design coupling matters.
🟠 Notable (inline)
runTailwindhangs forever ifpnpmisn't on PATH — noerrorhandler (build-tailwind-css.mjs:113).- Undocumented path coupling between
pipeline.config.jsonsources and the generated CSS location (pipeline.config.json:6). - New cross-package-boundary import from
docs/into../styles/tailwind(docs/postcss.config.mjs:1). - CJS
module.exportsin an ESM package (tailwind.config.js:2).
Additional findings
- Violates the issue's own acceptance criteria:
docs/content/bichat/index.mdx:334-336still contains the v3 directives@tailwind base; @tailwind components; @tailwind utilities;. Issue #648 explicitly requires "no remaining v3-only directives in migrated entry files," and the PR's verification checklist claimsrg "@tailwind base|@tailwind utilities"found nothing — so that search misseddocs/. This ships a broken v4 example to downstream applet authors. - No tests for the pipeline script.
createSourceBlock(dedup/sort),removeGeneratedSourceBlock(a lazy[\s\S]*?regex over CSS), and the--generate-onlyoutput are untested — and this script is the sole source of truth for the CSS the entire Go backend embeds. Add golden-file tests. build:css:watchinconsistency (package.json:7): it omits--minify, butjust css --watchalways prepends--minify— the two watch entry points produce different artifacts.- Stale docs:
docs/content/architecture/frontend-stack.mdx:191still describes the v3 model ("configured attailwind.config.js") and doesn't mentionpipeline.config.json/@source.
Process
- Don't leave DRAFT until CI is green — the PR description's
just css ✅is not a substitute forgo vet/build verification in CI, which is currently skipped.
| stdio: "inherit", | ||
| }); | ||
|
|
||
| child.on("close", (code) => { |
There was a problem hiding this comment.
🟠 Major (correctness) — runTailwind only listens for close, so a failed spawn hangs forever.
spawn() emits an error event (not close) when the binary can't be launched (e.g. pnpm not on PATH). This Promise only handles close, so in that case error fires, close never does, and the Promise never settles — a silent hang in CI and local builds.
Fix: add child.on("error", reject) alongside the close handler.
| "generated": "styles/tailwind/main.generated.css", | ||
| "output": "modules/core/presentation/assets/css/main.min.css", | ||
| "sources": [ | ||
| "../../cmd/**/*.{go,templ,html,js,ts,tsx}", |
There was a problem hiding this comment.
🟠 Major (design) — load-bearing, undocumented path coupling between this config and the generated CSS.
These sources entries are written verbatim as @source directives into styles/tailwind/main.generated.css. Tailwind v4 resolves @source relative to the CSS file's location (styles/tailwind/), not relative to pipeline.config.json. The script does no normalization — it passes the ../../... prefixes raw. This only works because the config and the generated CSS happen to share a directory; point --config at a config elsewhere and every ../../ source silently resolves to the wrong place.
Fix: resolve source paths relative to the config file and re-relativize to the generated output file, or at minimum document this constraint with a comment in the JSON.
| }, | ||
| }; | ||
| export default config; | ||
| import { createTailwindPostcssConfig } from "../styles/tailwind/postcss.shared.mjs"; |
There was a problem hiding this comment.
🟠 Major (design) — new cross-package-boundary import.
docs/ is a standalone Next.js package with its own package.json/node_modules. This reaches up and across into a sibling directory (../styles/tailwind/postcss.shared.mjs) that isn't declared as a dependency anywhere, so the coupling is invisible to tooling and only works because CI checks out the whole repo. postcss.shared.mjs is ~3 lines.
Fix: inline the config in docs/postcss.config.mjs, or publish postcss.shared.mjs as a proper importable package — don't introduce a silent cross-boundary import.
| @@ -1,10 +1,6 @@ | |||
| /** @type {import('tailwindcss').Config} */ | |||
| module.exports = { | |||
There was a problem hiding this comment.
🟡 Minor (sloppy) — CommonJS module.exports in a "type": "module" package.
package.json declares "type": "module", so .js files are ESM by Node's native resolver — but this file uses module.exports. Tailwind's jiti loader tolerates it today, but any tool loading it via native ESM will fail, and the downstream docs already recommend tailwind.config.cjs to consumers (docs/content/getting-started/npm-package.mdx:28).
Fix: rename to tailwind.config.cjs and update the @config references (styles/tailwind/input.css, modules/core/presentation/assets/css/main.css).
Summary
This PR completes Tailwind v4 generation-flow alignment in
iota-sdkand also introduces a canonical shared setup to reduce future drift.Fixes #648
What Changed
1) Canonical root pipeline
styles/tailwind/pipeline.config.json.scripts/build-tailwind-css.mjsto be the canonical pipeline entrypoint.--generate-only,--minify,--clean, and passthrough tailwind flags.pipeline.config.json.@sourcedirectives instyles/tailwind/main.generated.css.--generate-onlymode.Justfileto call only this script:just css->node scripts/build-tailwind-css.mjs --minify ...just css-clean->node scripts/build-tailwind-css.mjs --clean2) Shared PostCSS config wiring
styles/tailwind/postcss.shared.mjswithcreateTailwindPostcssConfig()helper.modules/bichat/presentation/web/postcss.config.jsto consume shared helper.docs/postcss.config.mjsto consume shared helper (withautoprefixer: false), so plugin wiring stays consistent across targets.3) Tailwind dependency/version drift controls
tailwindcss@4.1.18@tailwindcss/cli@4.1.18@tailwindcss/postcss@4.1.18pnpm.overridesin both root and BiChat webpackage.jsonto enforce those versions.4) Previous migration deltas retained
tailwind.config.jskeeps v4 model (removed v3contentblock for root flow).styles/tailwind/main.generated.cssremains generated-only (gitignored).Verification
node scripts/build-tailwind-css.mjs --generate-only✅just css✅pnpm run build:css(root canonical command path) ✅pnpm -C modules/bichat/presentation/web run build✅rg -n "@tailwind base|@tailwind components|@tailwind utilities" styles modules .github(no matches) ✅Environment-limited check
go vet ./...⛔ blocked locally by macOS/Xcode license prompt:You have not agreed to the Xcode license agreements...Risk / Rollback
Notes
../appletswas not necessary for this change because config-sharing and pipeline centralization are fully contained iniota-sdk.