Repository navigation
Conversation
dist/**/[A-Z]**/index.js and dist/**/[A-Z]**/[A-Z]*.js match nearly every component directory, so webpack treats almost the whole compiled package as side-effecting and won't drop unused barrel exports (e.g. Video, which drags in react-player's per-provider chunks even when unused). These globs replaced two explicit file entries in #932 (2020) to cover CSS Modules side effects. That migration to Emotion is done; src has no remaining *.module.css/scss files, and the one raw CSS import left (VidstackPlayer) is already covered by the **/*.css glob. Dropping the catch-all restores tree-shaking without losing real side-effect coverage. gamut-illustrations had the identical catch-all with no CSS imports at all, so it moves to sideEffects: false, matching gamut-icons/gamut-patterns. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
jakemhiller
approved these changes
Sep 15, 2026
jakemhiller
left a comment
jakemhiller
left a comment
Member
There was a problem hiding this comment.
worth testing the alpha package, but i think this will be fine
|
View your CI Pipeline Execution ↗ for commit 21bd648 ☁️ Nx Cloud last updated this comment at |
|
Collaborator
|
📬 Published Alpha Packages:
|
Contributor
|
🚀 Styleguide deploy preview ready! Preview URL: https://6aa9acdd9140e5af2515de55--gamut-preview.netlify.app |
Member
Author
|
closing this because it doesn't solve the core problem and it's superseded by the much more robust #3439 |
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
3 participants
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.
Overview
packages/gamut/package.json'ssideEffectsfield includes two catch-all globs,dist/**/[A-Z]**/[A-Z]*.jsanddist/**/[A-Z]**/index.js, which match nearly every component directory (all are capitalized). That makes webpack treat almost the entire compiled package as side-effecting, so it won't drop unused exports from the barrel (src/index.tsx) — includingVideo, which statically importsreact-player. A consumer that pulls in gamut via a Module Federation shared singleton (loading the barrel directly, not through its own imports) ends up withreact-player's per-provider chunks (DailyMotion, YouTube, Vimeo, etc.) in its bundle even when it never renders<Video>.These globs came from #932 (2020), which replaced two explicit entries (
dist/ButtonBase/index.js,dist/Card/CardShell.js) with the catch-all pattern. @jakemhiller's rationale: "all components that import CSS Modules have side-effects... going to keep popping up," flagging the approach as "kind of ugly" and expecting the then-in-progress Emotion migration to change the calculus later.That migration is done.
srchas zero*.module.css/*.module.scssfiles today, and the one remaining raw CSS import (Video/lib/VidstackPlayer/index.tsx) is already covered by the separate**/*.cssglob. The premise for the catch-all no longer holds, so this drops it and keeps only the CSS/SCSS globs — the same explicit-entry model #932 moved away from, minus the problem that motivated the move.gamut-illustrationshad the identical catch-all with no CSS imports anywhere in its source, so it moves tosideEffects: false, matchinggamut-icons/gamut-patterns.PR Checklist
Testing Instructions
yarn nx build gamutandyarn nx run gamut:verify— both pass (build output and type-check unaffected bysideEffectsmetadata itself).sideEffectsonly affects consumer bundler behavior, not this repo's own build. Verify in a downstream consumer: run its production webpack build before/after bumping to this version and confirm thereactPlayer*provider chunks disappear when<Video>isn't used.mainto confirm no component regresses from losing the catch-all — a regression would mean it has a real eval-order dependency independent of CSS Modules, and needs an explicit entry added back (same model as the pre-fix: add sideEffects globs that catch all components #932ButtonBase/CardShellentries).Jira
GMT-1788