Fix: make published dist tree-shakeable - #191
Conversation
The barrel entries (./ui, ./remix-hook-form) welded ~200 KB gzip of
date-fns, react-day-picker, cmdk, and input-otp onto every consuming
chunk: Vite lib-mode chunking merged components into shared hash chunks
and hoisted their transitive externals into each entry stub as bare
side-effect imports (dist/ui/checkbox.js opened with ~40 of them).
- Declare "sideEffects": false (no CSS or side-effectful modules ship)
- Build with preserveModules + hoistTransitiveImports: false so each
source module maps to exactly one dist module with only its own imports
- Externalize all bare specifiers instead of a hand-maintained list;
libphonenumber-js and react-stately were silently bundled because the
list missed them - they are now declared dependencies
- Import cn from './utils' instead of the '../ui' barrel in checkbox,
dialog, slider, tabs; import useDataTableFilters from its module
instead of the data-table-filter barrel in data-table-router-form
- Drop the remix-hook-form regular dependency (it stays a peer, widened
to ^7.1.0) so consumers no longer resolve a second nested copy
Bundling `import { Checkbox } from '@lambdacurry/forms/ui'` with
esbuild: 350 KB package code + date-fns/react-day-picker/cmdk/input-otp
before, 6.6 KB and none of them after. All previous dist module paths
still exist, so deep subpath imports are unaffected.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. WalkthroughThe package metadata and Vite build now produce a tree-shakeable distribution. Runtime and peer dependencies are updated, internal imports use direct module paths, and release notes document the changes. ChangesTree-shakeable distribution
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR improves published bundle tree-shaking and updates dependency declarations without any actionable merge-blocking risk remaining after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant ViteConfiguration
participant Rollup
participant PackageConsumer
ViteConfiguration->>Rollup: classify bare npm imports as external
ViteConfiguration->>Rollup: preserve modules and disable import hoisting
Rollup->>PackageConsumer: emit individual package modules
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 7 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Pre-existing on main; blocks the lint check on any PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
📝 Storybook Preview: View Storybook This preview will be updated automatically when you push new changes to this PR.
|
1 participant
Problem
Consumer bundlers cannot tree-shake
@lambdacurry/forms. Importing a single component from the./uior./remix-hook-formbarrels drags ~200 KB gzip of date-fns, react-day-picker, cmdk, and input-otp into the consuming chunk. In the 360training monorepo the checkout app had to migrate every import to deep subpaths (@lambdacurry/forms/remix-hook-form/text-field) to work around it.Two root causes:
Checkboxlived infilter-selector-DCKG1gjI.js) and hoisted the chunks' transitive externals into every entry stub as bare side-effect imports —dist/ui/checkbox.jsopened with ~40 lines likeimport "date-fns",import "react-day-picker",import "input-otp". Bundlers must preserve bare imports, so tree shaking was defeated.sideEffectsdeclaration, so bundlers had to assume every module matters.A few source files also imported
cnfrom the../uibarrel (checkbox, dialog, slider, tabs), putting the entire barrel graph into those components' module graphs.Changes
"sideEffects": false— the package ships no CSS and no side-effectful modules.preserveModules+hoistTransitiveImports: false: each source module now maps to exactly one dist module containing only its own imports. No more hash chunks or facade stubs.cmdk,@tanstack/react-table, and several Radix packages were being bundled into dist, andlibphonenumber-js/react-statelywere bundled while missing fromdependenciesentirely (now declared).remix-hook-formwas both a regular dependency (exact7.1.0) and a peer — consumers could resolve a second nested copy alongside their own (360training currently resolves both 7.1.0 and 7.1.1). It is now peer-only, widened to^7.1.0, with a devDependency for local development. Note for Yarn/pnpm consumers: if you don't already listremix-hook-formin your dependencies, add it (npm auto-installs peers).cnfrom./utils,useDataTableFiltersfrom its own module.Verification
esbuild bundle of
import { Checkbox } from '@lambdacurry/forms/ui'with all npm deps external:import { TextField } from '@lambdacurry/forms/remix-hook-form': 357 KB → 9.4 KB, no heavy deps.grepconfirms zero bare side-effect imports remain anywhere in dist.yarn build(components + docs Storybook),type-check, and Biome on changed files all pass. (yarn lintfails onscripts/release-if-needed.mjsformatting — pre-existing on main from 2e542e0.)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores