Conversation
…ut-tests
None of these three packages declared a `sideEffects` field, which is the
most conservative default: webpack must assume every module in the barrel
is side-effectful and cannot drop unused exports. That currently applies to
the packages everything else depends on.
Verified safe to mark `sideEffects: false`:
- no module-level `document`/`window` access or `injectGlobal` in either
package's source
- gamut-styles' globals/{Reboot,Variables,Typography}.tsx and
GamutProvider.tsx use Emotion's `<Global>` component, which runs at
render time inside React, not at module evaluation
- AssetProvider.tsx's createFontLinks is an exported function, not a
module-eval call
This is PR 0a of the tsdown migration plan (GMT-1741) — a standalone fix
for a live tree-shaking bug on the current Babel build, landing ahead of
the build-tooling change. gamut and gamut-illustrations carry a separate,
deliberately-added sideEffects glob (added in #932 to fix real style-order
drift) that needs a CSS-diff experiment before it can be safely removed;
that's scoped to PR 0b.
Verified: `yarn build`, `jest --selectProjects gamut-styles variance
gamut-tests` (181/181 passing), and `yarn build-storybook` (production
webpack build) all succeed unchanged.
Replaces `babel ./src --out-dir ./dist` with `tsdown` for JS, keeping the
existing `tsc --emitDeclarationOnly` step for declarations (the hybrid).
Chain must migrate atomically (variance -> gamut-styles -> gamut) — a
half-migrated chain doesn't load, because Babel's ESM output has
extensionless relative specifiers that Node's own resolver rejects.
- dts: false in every tsdown.config.ts; tsc keeps emitting declarations
into the same dist/ (clean: false in tsdown, single `rm -rf ./dist` at
the head of the build target owns cleaning). Bundled tsdown
declarations were tried and rejected: 50x TS4023 downstream, since a
bundled .d.ts doesn't re-export names its dependents need.
- No external/deps.neverBundle needed for variance or gamut-styles —
tsdown auto-externalizes anything in a package's own `dependencies` /
`peerDependencies`, which already covers everything they import.
gamut needs one exception: `deps.neverBundle: [/\.css$/]`, since tsdown
refuses to bundle a relative .css import without @tsdown/css, and
externalizing it matches the current cpy-based approach exactly.
- Output naming: since none of these packages set `"type": "module"`,
tsdown names CJS `dist/index.js` and ESM `dist/index.mjs` (not
`.cjs`/`.mjs` as sketched in early spike notes) — package.json
`exports`/`main`/`module`/`types` updated to match the real filenames.
Kept `main`/`types` accurate (not just `exports`), since this repo's
tsconfig.base.json uses legacy `moduleResolution: "node"`, which
ignores `exports` entirely for internal cross-package typechecking.
- gamut-styles: added the missing `types` field and dropped `files`
entries (core, utils, core.scss, utils.scss) that don't exist on disk.
- gamut: sideEffects reduced to `["**/*.css", "**/*.scss"]` (the two
`dist/**/[A-Z]**/*.js` globs from #932 are now dead — they matched
gamut's old per-file dist shape, which no longer exists). Re-verify
with the CSS-diff + esbuild single-import probe from the sideEffects
investigation once the leaf packages (icons/patterns/illustrations)
are also migrated, since gamut's own barrel depth was masking whether
this field does anything until now.
- BarChart/BarRow/elements.tsx: `@emotion/babel-plugin` stamps a `target`
on every styled() call unconditionally, and `target` is what makes the
`${CategoryLabel}` component-selector interpolation work. The plugin
only runs on the babel path now (jest/Storybook loader), not through
tsdown, so this is the one site that needs an explicit target to avoid
a silent `.undefined` fallback in the production bundle. Verified in
the built dist/index.js: `target: "gmt-category-label"` survives.
- tsconfig.base.json: isolatedModules: true, ratcheting against the
type-only re-export debt rolldown can't see through (verified 0 errors
across every build-relevant package before enabling).
- gamut build:watch now runs `tsdown --watch` instead of a full rebuild
on every source change (watch mode is otherwise unverified — worth
confirming latency once used for real).
Verified: `yarn nx run-many --target=build --projects=variance,gamut-styles,gamut`,
full jest suite for all four affected packages (1457/1457 passing),
`yarn verify` (isolatedModules didn't break typecheck anywhere), and
direct CJS `require()` of variance's and gamut-styles' dist output.
gamut's own CJS load still fails via require() — but only because it
transitively depends on gamut-icons, which is still Babel-built and has
a pre-existing (unrelated) ERR_UNSUPPORTED_DIR_IMPORT under Node's
require(esm); confirmed identical failure on the untouched dist before
this change. Resolves once gamut-icons migrates next.
Part of the tsdown build migration (GMT-1741).
…ns, gamut-tests
Same hybrid pattern as the previous commit (tsdown for JS, tsc for
declarations), applied to the four remaining Babel-built packages. Nothing
depends on these, so they could lag the dependency chain — but since
gamut itself depends on gamut-icons/gamut-patterns/gamut-illustrations,
finishing them now is what actually resolves gamut's own CJS load (the
previous commit's ERR_UNSUPPORTED_DIR_IMPORT was gamut-icons transitively,
not gamut).
SVGR steps (icon/pattern codegen, the `-icon.svg` rename) are untouched —
tsdown/rolldown only replaces the babel step in each pipeline.
## unbundle: true, added to every package (retroactively, including the
## previous commit's three)
A single-component import probe (`import { FillButton } from
'@skillsoft/gamut'`, bundled with esbuild) found the default tsdown output
— one concatenated dist/index.mjs per package, root-only entry — pulls in
100% of that file's ~428KB plus every heavy dependency any of gamut's ~130
exports uses transitively (@vidstack/react, gamut-icons' whole barrel,
react-aria-components, framer-motion, @formatjs/*, react-select): 1.59MB
for one button, 15x today's Babel-build baseline of 103KB. Same problem on
gamut-icons: importing one icon pulled in all 377 (586KB).
`unbundle: true` (tsdown mirrors src/ file-by-file instead of
concatenating) fixes it completely — 112KB for the gamut probe, 73KB for
the icon probe, both at or below the Babel baseline — while keeping
tsdown's real relative-import extensions (.mjs/.js, not Babel's
extensionless ones) and the speed win. Applied to all seven packages for
consistency; the failure mode is structural (one shared module scope), not
size-dependent, so there's no reason to leave any package on the default.
One benign side effect, checked: unbundle's finer-grained CJS module graph
surfaces a pre-existing circular import in Tip's style utilities as a
Node "accessing non-existent property inside circular dependency" console
warning. Confirmed not a functional bug — the values are only read inside
function bodies, never captured at module-eval time, so by call time the
cycle has resolved; verified by requiring the built module directly and
calling it, real style values came back.
## Storybook deep imports into /src broke — and revealed Storybook doesn't
## actually consume any package's dist
packages/styleguide/.storybook/main.ts already aliases every bare
`@skillsoft/gamut*` specifier straight to that package's `../src`
directory (exact-match, `$`-anchored). That means Storybook has never
consumed built dist output, for any package, independent of this
migration — it only compiles raw TypeScript source through webpack. The
"Storybook proves the dist/exports map works" framing from earlier in
this migration doesn't hold; it only incidentally caught real breaks
because a handful of imports carry an extra `/src` path segment that
doesn't match the bare-specifier alias, so those specific ones fall
through to real node_modules -> exports resolution.
13 such imports broke once the exports map went strict:
- 7 were lazy — the symbols they wanted are already exported at the
package root (theme, trueColors, Box, FillButton, etc). Fixed by
importing from the root, matching the codebase's own
`@skillsoft/gamut/import-paths` eslint rule.
- 6 reach into genuinely internal groupings with no public equivalent
(icon categories, raw typography variant metadata, the full
system-props registry for a docs table) — each already carried an
acknowledged `// eslint-disable-next-line @skillsoft/gamut/import-paths`
comment, pre-existing debt this migration surfaced rather than
introduced. Fixed with four new Storybook-only webpack aliases pointing
at the same monorepo source paths, keeping every package's `exports`
map an honest description of its published surface instead of growing
it to fit docs tooling.
Verified: full `yarn build`/`yarn test`/`yarn verify` (10 projects, 1276
tests), direct CJS `require()` of all seven packages' dist output
(matching export counts throughout), `yarn build-storybook` +
`nx run styleguide:storybook-test` (87 suites / 493 tests, including the
icon galleries and typography-variant table the new aliases unblock), and
a 493-story Emotion-CSS diff across sideEffects on vs. off — byte-identical
(0 differences), now under real tree-shaking (unbundle mode), confirming
the 2020 style-order bug (#932) genuinely doesn't reproduce under Emotion
11, not just that it was unreachable to test.
Part of the tsdown build migration (GMT-1741).
…olver, lint rule)
Closes out the remaining PR 1 verification scaffolding from the tsdown
migration plan (GMT-1741): the automated guard that substitutes for
having real published consumers to catch a broken exports map.
## verify-package nx target (publint + attw), all seven packages
Added `publint .` and `attw --pack . --ignore-rules false-cjs` as a new
`verify-package` target, depending on `build`, on every tsdown-migrated
package.
`--ignore-rules false-cjs` suppresses one accepted, deliberate tradeoff
of the hybrid build: attw flags "masquerading as CJS" because a single
shared `dist/index.d.ts` serves both the `import` and `require`
conditions (tsc only emits one declaration tree, not per-format
`.d.mts`/`.d.cts` copies). That only affects `moduleResolution: node16`/
`nodenext` consumers importing via ESM syntax — this repo's own
tsconfig.base.json uses legacy `moduleResolution: "node"`, and the
bundler profile (the actual consumption path) is unaffected. Splitting
declarations per format to satisfy this would mean tsc emitting twice
the output for a benefit no current consumer needs.
Tried adding `"type": "commonjs"` per publint's own suggestion (removes
a minor Node startup perf-hit note) and had to revert it immediately —
it broke tsdown's own ability to load `tsdown.config.ts`, which uses ESM
`import` syntax and lives in the same directory. Every package's config
file depends on the "type" field staying unset; verified by testing the
change, watching `variance:build` fail with "Cannot use import statement
outside a module", and reverting across all seven packages.
## script/verify-exports.mjs
Resolves every package's `exports` map from the repo root the way a real
consumer would: asserts the root subpath resolves AND loads via both
`require()` and `import()`, and asserts old deep `dist/*` paths are
blocked with ERR_PACKAGE_PATH_NOT_EXPORTED.
The ESM `import()` check surfaced a real, pre-existing gap: variance (and
everything depending on it) imports lodash via extensionless deep
subpaths (`lodash/get`, not `lodash/get.js`). lodash ships no `exports`
map, so Node's native ESM resolver — unlike `require()`, and unlike every
bundler's resolver (webpack/esbuild/rollup all probe extensions) —
refuses it outright. This is inherent to lodash's own packaging, predates
this migration, and only matters for a consumer running raw Node ESM
with no bundler in front, which isn't a real path for any package here.
Fixing it means rewriting ~25 files' lodash imports across 5 packages;
scoped out as a fast-follow rather than blocking here. The script treats
it as a clearly-flagged warning, not a failure.
## eslint rule: require-styled-target-for-selector
Codifies the BarChart `target` fix from the first tsdown-migration commit
as an enforced rule, added to `recommended.ts`. Flags a `styled()` call
used via `${Component}` interpolation in a computed style-object key
that has no explicit `target` — the exact silent-`.undefined`-in-production
failure mode `@emotion/babel-plugin` used to paper over automatically.
Verified against the real gamut/gamut-styles/gamut-icons/gamut-patterns/
gamut-illustrations source with `--no-inline-config` (ignoring existing
disable comments): zero violations, confirming BarChart was the only
site and there's nothing else to fix.
## CI
Wired `verify-package` and `verify-exports.mjs` into the storybook-test
job's build step, ahead of the Storybook build.
Verified: full `yarn build`/`yarn test`/`yarn verify`, `yarn nx run-many
--target=verify-package --all` (7/7 clean), `node script/verify-exports.mjs`
(all checks passed, lodash gap flagged not failed), and the new eslint
rule's test suite (60/60 passing across the whole plugin).
Part of the tsdown build migration (GMT-1741).
… into cass-gmt-1741 # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
The root tsconfig.json (not used by any real build — every package
verifies against its own scoped tsconfig via cwd) is what editors like
Zed fall back to for files no package-scoped tsconfig covers, which
today means tsdown.config.ts / tsdown.base.ts. Two problems stacked:
- The literal `.ts` extension on `import { baseConfig } from
'../../tsdown.base.ts'` (required because tsdown's own config loader
needs it) needs `allowImportingTsExtensions`, which itself requires
`noEmit` or `emitDeclarationOnly`.
- tsdown itself ships no `main`/`types` fields, only a modern `exports`
map, so it's completely unresolvable under the inherited
`moduleResolution: "node"` — confirmed via tsc's own error message.
Scoped to this one root config, not tsconfig.base.json (which every
package inherits) — verified no real build/verify script uses the bare
root config, so this only affects editor diagnostics for the tsdown
config files, with zero effect on any package's real typecheck.
Part of the tsdown build migration (GMT-1741).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # packages/styleguide/.storybook/main.ts # yarn.lock
…babelrc All 7 packages' babel.config.js had a byte-identical `presets` array (preset-env + preset-react + preset-typescript), differing only in whether @emotion/babel-plugin was added. Babel's own `extends` merges arrays rather than overriding them, so moving the shared presets into babel.defaults.js (which every package already extends, for its `ignore` patterns) lets each package's own config shrink to just what's actually different for it — the emotion plugin, or nothing at all. Verified the merge behavior directly rather than assuming it: ran @babel/core's transformFileSync against a real gamut-icons component (JSX transform + TS stripping both applied) and a real gamut-styles styled() call (target + label stamping both present) with the package's own config reduced to bare `extends`. Confirmed with the full suite too: build/test/verify all green, 117 test suites / 1563 tests with jest cache cleared, and the BarChart Emotion-target site specifically re-checked given its history this migration. Also dropped gamut-tests' bogus top-level `include: ['./src/**/*']` key while simplifying that file — a no-op given babel's only ever invoked against files already under ./src. Removed packages/styleguide/.babelrc.json — genuinely dead now that Storybook's Vite migration (main #9) wires @emotion/babel-plugin directly via @vitejs/plugin-react's babel option in .storybook/main.ts, rather than reading this file. Confirmed nothing else discovers it (no jest config references styleguide; it has no test target). Part of the tsdown build migration (GMT-1741). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Storybook's Vite preview now uses @vitejs/plugin-react-swc + @swc/plugin-emotion instead of @vitejs/plugin-react + @emotion/babel-plugin, removing Babel from Storybook's build pipeline. Speeds up the dev-loop HMR turnaround (~3-5x faster per-edit in local measurement); cold build-storybook time is roughly a wash. @vitejs/plugin-react-swc, @swc/plugin-emotion, and the @swc/core resolution are pinned to versions from the same release window (June-July 2026) rather than latest - newer combinations crash with an undocumented WASM/ABI mismatch between the plugin and swc_core. Bump these three together. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Keep the Storybook SWC Emotion plugin from this branch; take main's prettier/lint formatting and Storybook interaction/a11y test updates. Co-authored-by: Cursor <cursoragent@cursor.com>
@skillsoft/eslint-plugin-gamut
@skillsoft/gamut
@skillsoft/gamut-agent-tools
@skillsoft/gamut-icons
@skillsoft/gamut-illustrations
@skillsoft/gamut-patterns
@skillsoft/gamut-styles
@skillsoft/gamut-tests
@skillsoft/variance
commit: |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
… into cass-gmt-1795
# Conflicts: # yarn.lock
This branch has not been deployed
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
1 participant
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
Storybook's Vite preview build now reaches for
@vitejs/plugin-react-swc+@swc/plugin-emotioninstead of Babel for Emotion's readable class names / source maps — Babel is otherwise gone from
this repo since the tsdown migration. Since that swap landed, this PR also fixes a latent
type-only-import bug the swap exposed (Storybook's preview silently crashing on load) and closes
the gap so it can't recur, plus syncs the branch with
main.Babel → SWC for Emotion (original change)
packages/styleguide/.storybook/main.ts— swaps Babel's@emotion/babel-pluginfor@vitejs/plugin-react-swc+@swc/plugin-emotion; Storybook's build no longer touches Babel.one line in a story file and timing to Vite's
page reloadlog line).yarn build-storybooktime is roughly a wash (15.6–21.2s vs 18.3–18.5s Vite build step).autoLabel/labelFormatparity confirmed — generated classes still come out asgamut-<hash>-<ComponentName>, matching the Babel plugin's output shape.@vitejs/plugin-react-swc,@swc/plugin-emotion, and@swc/coreare pinned to the same release window (June–July 2026);the latest of each (Sept 2026) crashes with an opaque WASM panic on real
.stories.tsx/.mdxfiles — an undocumented ABI mismatch
plugins.swc.rs's compatibility lookup doesn't have datafor yet. Left a comment in
main.tsso a routine dependency bump doesn't silently reintroducethis.
Type-only import safety (new — fixes Storybook silently failing to load)
packages/gamut/src/BarChart/index.tsx— root cause ofyarn start:storybookloading thesidebar but never rendering any story content.
BarChartSingleValueBarSummaryContext,BarChartStackedSummaryContext,BarChartProps,BarProps, andInferBarTypeare type-onlyexports, imported here as regular (value) imports and referenced only inside an
export type { ... }re-export. SWC's per-file type-elision heuristic (same limitation Babeland esbuild share — no transpiler here has full type info) kept them as real runtime imports;
the browser's module loader then failed to link against
shared/translations.ts/shared/types.ts,which only export types, crashing the whole preview iframe before any story could render.
gamut-icons(icon-template.js) andgamut-patterns(
pattern-template.js) codegen templates, and in ~400 hand-written files acrossgamut,gamut-illustrations,gamut-styles,variance, andstyleguide— all mechanically fixed bysplitting type-only names into
import type { ... }..eslintrc.js— adds@typescript-eslint/consistent-type-imports(error,fixStyle: 'separate-type-imports',disallowTypeAnnotations: false) repo-wide so this classof bug fails lint instead of crashing silently at runtime. (Tried enforcing this via
tsconfig.base.json'sverbatimModuleSyntaxinstead — reverted; it also governsts-node'sresolution for every
jest.config.tsand the CJS-compiledeslint-plugin-gamutbuild, both ofwhich need
tscto rewrite ESM syntax to CJS, whichverbatimModuleSyntaxforbids. The lintrule gives the same "can't merge broken code" guarantee without that collateral breakage.)
packages/eslint-plugin-gamut/package.json— drops the"module"field; it pointed at thesame CJS file as
"main"(no separate ESM build backs it), so it was misleading rather thanuseful to bundlers that treat
"module"as an ESM entry signal.PR Checklist
new lint rule (repo-wide, not story-specific); no new runtime tests added
Testing Instructions
yarn build-storybooksucceeds, including all.mdxdoc pages.yarn test:storiesandnpx nx run gamut:testpass (1276 tests, gamut + its 6 build deps).yarn start:storybook— dev server starts clean; open any story (e.g.Organisms/BarChart,Foundations/Utilities) and confirm content renders with noconsole/page errors (previously: sidebar loaded, content pane spun forever).
yarn lintpasses with the new@typescript-eslint/consistent-type-importsrule active.autoLabel: 'always'/labelFormat: '[local]'parity.BarChart/elements.tsxexplicittarget: 'gmt-category-label'call site unaffected (it's astyled()runtime arg, not transform output).verbatimModuleSyntaxrevert rationale before merge.