fix: tailwind-merge runtime dep, Button API docs, and a smoke test that replaces attw - #212
Merged
Merged
Conversation
127 shipped dist files `import { twMerge } from "tailwind-merge"`, but the
package declared it only as a devDependency. A consumer installing
@pathscale/ui into a project that does not already have tailwind-merge
gets a runtime failure on first import:
Cannot find package 'tailwind-merge' from
.../@pathscale/ui/dist/components/button-group/ButtonGroup.js
It went unnoticed because every app in this org installs tailwind-merge
itself, so the import resolves from the app's own node_modules. A fresh
consumer has nothing to resolve it from.
Found by the consumer smoke test on its first run, against the tarball
published as 1.3.0 twenty minutes earlier.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ui-usage.md and the README both showed <Button color="primary" size="md"> Button has no `color` prop. The shipped type takes `variant` (primary | secondary | tertiary | outline | ghost | danger | danger-soft) and `size` (sm | md | lg) — narrower than the documented xs | sm | md | lg | xl. The blanket claim that sizes and colors are shared unions was also wrong in practice: ComponentSize and ComponentColor are declared in src/components/types.ts but never re-exported from the root barrel, so consumers cannot import them, and components narrow them anyway. `color` exists on eleven components; `variant` on about forty. The npm page has been telling people to write code that does not compile. I rewrote that README verifying every path and never once verifying a prop, and it propagated the same error from ui-usage.md. Also drops the warning about README paths that do not exist; both were fixed in 1.3.0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@arethetypeswrong/cli modelled four TypeScript resolution modes, three of which nothing here ships to, and reported 378 findings. The split was clean: 200 extensionless-import errors under node16 only, and 178 CSS side-effect imports under node10 and bundler. Every consumer app in this org sets skipLibCheck: true, so none of it was ever felt. Zero actionable findings. This installs the packed tarball into a fixture app configured the way those apps are — bun, moduleResolution: bundler, solid-js, peer deps derived from the manifest — then typechecks it and imports it at runtime. On its first run it caught tailwind-merge being a devDependency while 127 shipped files import it, and that Button takes `variant` rather than the documented `color`. attw found neither in 378 lines of output. Advisory for now. Today's release failed four times on gates that had never executed in CI; this one gets promoted to blocking only after it has run green a few times. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
🚀 Preview deployment is ready! You can view the preview at: https://pr-ui-preview-212.surge.sh |
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.
You asked whether something better than attw exists. Yes — and it found three real bugs on its first run, two of them in the 1.3.0 I published twenty minutes earlier.
1.
tailwind-mergeis a devDependency, and the package needs it at runtime127 shipped
distfiles doimport { twMerge } from "tailwind-merge". It was declared only indevDependencies. A consumer installing into a project that doesn't already have it:Invisible in our own apps because every one of them installs
tailwind-mergeitself, so the import resolves from the app'snode_modules. A fresh consumer has nothing to resolve it from. The package is broken on install today.2. The documented Button API does not compile
ui-usage.mdand the README both showed<Button color="primary" size="md">. The shipped type:No
colorprop at all, and the size union is narrower than the documentedxs | sm | md | lg | xl.ComponentSizeandComponentColorare declared insrc/components/types.tsbut never re-exported from the root barrel, so consumers can't import them either. In practicecolorexists on eleven components andvarianton about forty — there is no shared union to document.I rewrote that README verifying every path and not one prop, and propagated the error from
ui-usage.mdstraight onto the npm front page. Corrected in both places.3. attw was the wrong tool, replaced
378 findings, split perfectly cleanly:
moduleResolution.cssside-effect importsnode10node16bundlerEvery consumer app in this org sets
skipLibCheck: true, so none of it is ever felt. Zero actionable findings, and nothing about Node runtimes — those are TypeScript resolution modes.Replaced with a consumer smoke test: pack the tarball, install it into a fixture app configured the way our apps are (bun,
moduleResolution: bundler, solid-js, peer deps derived from the manifest), then typecheck and import it. It caught both bugs above; attw caught neither in 378 lines.It takes
SMOKE_TARBALL=/path/to.tgzso it can be run against any published version — that is how I verified it against the live 1.3.0 rather than wiring another untested gate into the release path.Advisory, not blocking. Today's release failed four times on gates that had never executed in CI. This gets promoted once it has run green a few times.
Release
Merging publishes 1.3.1 automatically — two
fix:commits, so a patch bump. That release is the one where a freshbun add @pathscale/uiactually works.🤖 Generated with Claude Code