fix(styles): ship tw-animate-css as a runtime dependency - #16
Merged
Conversation
globals.css is published and re-parsed inside the consumer's Tailwind build, so every bare specifier it names resolves from the consumer's node_modules. tw-animate-css was a devDependency, so it never reached them and `@import "@fx/ui/styles/globals.css"` failed outright with "Can't resolve 'tw-animate-css'" — the documented integration path did not work for anyone. Promote it to a runtime dependency, the same treatment @tailwindcss/typography and @fontsource/jetbrains-mono already get for the same reason. Add a test that reads globals.css and asserts every package it imports is declared as a runtime dependency, so the next addition cannot repeat this. Tailwind itself is exempt: it is the compiler running the build, so it is necessarily present and cannot be supplied from here.
There was a problem hiding this comment.
Pull request overview
This PR fixes consumer Tailwind compilation failures by ensuring tw-animate-css is installed transitively when consumers import @fx/ui/styles/globals.css, and adds a regression test to prevent future globals.css imports/plugins from being left in devDependencies.
Changes:
- Promotes
tw-animate-cssfromdevDependenciestodependenciesso consumer builds can resolve it. - Adds a Vitest guard that parses
src/styles/globals.cssfor@import/@pluginbare specifiers and asserts they’re runtime dependencies. - Documents the shipped animation utilities and clarifies why
Collapsiblekeeps local keyframes.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/styles/__tests__/globals.test.ts |
New regression test to ensure globals.css’s external imports/plugins are runtime dependencies. |
README.md |
Documents animation utilities and the Collapsible keyframes rationale. |
package.json |
Moves tw-animate-css to runtime dependencies so consumers can resolve it. |
bun.lock |
Lockfile update reflecting the dependency move. |
Suppressed comments (1)
src/styles/tests/globals.test.ts:50
- Same issue as above for
devDependencies:Object.keys(packageJson.devDependencies)will throw if the field is missing. Default to{}to keep failures actionable.
it.each(referencedPackages())('does not leave %s in devDependencies only', (name) => {
expect(Object.keys(packageJson.devDependencies)).not.toContain(name)
})
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Derive the test's own directory from import.meta.url rather than __dirname, so reading globals.css does not depend on the runner supplying CommonJS globals to an ESM package. Default the manifest lookups to an empty object as well, so a missing dependencies field fails on the assertion naming the undeclared package instead of throwing a TypeError that hides it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
@import '@fx/ui/styles/globals.css'— the documented integration path — fails to compile for every consumer:globals.cssis published and re-parsed inside the consumer's Tailwind build, so every bare specifier it names resolves from the consumer'snode_modules.tw-animate-csswas declared indevDependencies, so it never reaches them.The fix
Promote it to a runtime dependency — the same treatment
@tailwindcss/typographyand@fontsource/jetbrains-monoalready get, for exactly the same reason.globals.cssis unchanged.Verified end to end against a packed tarball
bun pm packon this branch, installed into a clean consumer that declares notw-animate-cssof its own, then compiled with the Tailwind CLI. Same harness, only the@fx/uiversion differs:1.0.0tw-animate-cssin consumernode_modules@tailwindcss/clicompileCan't resolve 'tw-animate-css'The compiled output carries the utilities the import exists to provide —
animate-in,fade-in,zoom-in-95,slide-in-from-top-2— alongside the theme tokens,proseclasses, and JetBrains Mono.Regression guard
src/styles/__tests__/globals.test.tsreadsglobals.css, extracts every package it@imports or@plugins, and asserts each is declared as a runtime dependency and not left indevDependencies. The next thing added toglobals.csscannot repeat this.Against the current
mainit fails on preciselytw-animate-cssand passes@fontsource/jetbrains-monoand@tailwindcss/typography, so it is a verified guard rather than an assumed one.Tailwind itself is exempt via a documented allowlist: it is the compiler running the build, so it is necessarily already installed and cannot be supplied from here. A
references at least one external packageassertion guards the guard, so a regex that silently stops matching cannot turn the suite into a no-op.Note on Collapsible
Worth recording, since it is easy to "clean up" by mistake:
globals.cssdeliberately defines its owncollapsible-down/collapsible-upkeyframes against Base UI's--collapsible-panel-height.tw-animate-cssbundles keyframes of the same name targeting Radix, Bits, and Reka variables, which would not work here. The local definitions must stay. Documented in the README alongside the animation utilities.Test plan
bun run test— 476 tests across 35 files,Type Errors: no errorsbun run lint— matches themainbaselinebun run build— clean1.0.0(above)