ENG-3410: Drop Sass :export, create TS palette, enable Turbopack#7956
ENG-3410: Drop Sass :export, create TS palette, enable Turbopack#7956gilluminate merged 9 commits intomainfrom
Conversation
…vacy-center The fidesui palette is available both as a JS import (palette.FIDESUI_*) and as CSS variables (var(--fidesui-*)). This replaces all 40 JS imports with CSS variable equivalents, reducing build-time SCSS module resolution dependencies and aligning with the CSS-first direction of the codebase. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Drop the webpack-dependent :export block from palette.module.scss by creating a plain palette.ts with the same hex values. Theme files (default-theme, dark-theme, FidesUITheme, stories) now import from TS. Also: fix broken --fidesui-font-size-md var in ToastLink, migrate CustomTooltip to CSS vars, and point Tailwind neutral scale at CSS vars instead of hardcoded hex. ENG-3410 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The :export block in palette.module.scss was the only thing requiring webpack over Turbopack. Now that it's removed, drop the --webpack flags from all next build/dev scripts. Build time drops from ~1m44s to ~31s. ENG-3410 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Dependency Review✅ No vulnerabilities found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
There was a problem hiding this comment.
Code Review: ENG-3410 — Drop Sass :export, create TS palette, enable Turbopack
This is a clean, focused infrastructure migration. The goal is sound and the implementation is mostly correct. A few things worth calling out:
Notable Finding: Runtime Guard Removed in CustomTag.tsx
The old brandColor logic included a runtime key-existence check (FIDESUI_BG_${color.toUpperCase()} in palette) that would fall through to Ant's native color system for unknown values. The new code unconditionally generates a CSS var string for any string color prop.
This is safe under TypeScript because color is typed as BrandColor (an exhaustive union of CUSTOM_TAG_COLOR values), and all of those enum members have corresponding --fidesui-bg-* CSS variables in global.scss. However, it's a silent removal of a runtime safety net — any JS caller passing an arbitrary string (via an as BrandColor cast) will now get a silently broken background. A brief comment documenting the reliance on the BrandColor type would help (see inline comment).
Minor Issues
-
FIDESUI_NEUTRAL_75gap in Tailwind:palette.tsdefinesFIDESUI_NEUTRAL_75andglobal.scssgenerates--fidesui-neutral-75, buttailwind.config.jsmaps onlyneutral-1throughneutral-10(50–900), skipping 75. This meansbg-neutral-75won't work in Tailwind utility classes. Pre-existing gap, but more visible after this change. -
Sync comment in
palette.module.scssshould mentiontailwind.config.jsas a third location that needs to stay in sync, not justpalette.ts. -
palette.tsJSDoc doesn't document the key-naming convention (SCREAMING_SNAKE_CASE ↔ kebab CSS var withfidesui-prefix). Easy win for future contributors adding colors.
Positive Signal
- Removing the Sass
:exportblock is the correct architectural direction — it was a webpack-specific hack that blocked Turbopack adoption. - CSS variable migration in
CustomTag.module.scssandCustomTooltip.module.scssis clean and idiomatic. ToastLink.module.scss: switching fromvar(--fidesui-font-size-md)(undefined) tovar(--ant-font-size)is a correct fix.- The
palette.tsas constassertion correctly infers literal types rather thanstring. - The ENG-3479 sync comment is honest and appropriately scoped.
- Removing the
--webpackflags and their explanation comment is satisfying to see.
🔬 Codegraph: connected (46845 nodes)
💡 Write /code-review in a comment to re-run this review.
The legacyRootApi: true was telling Storybook 10 to use the React 16 rendering shim, which doesn't have unmountComponentAtNode in React 18.
|
@lucanovera fixed storybook. It just had a "legacy" config option that was no longer needed. |
lucanovera
left a comment
There was a problem hiding this comment.
Storybook fixed now with palette showing. Ship it!

Ticket ENG-3410
Description Of Changes
Completes the palette migration by removing the Sass
:exportblock frompalette.module.scss(which required webpack's CSS modules interop) and replacing it with a plain TypeScript module. This unblocks removing the--webpackflags from all Next.js scripts, enabling Turbopack builds (~31s vs ~1m44s).Also fixes a few issues discovered during the migration:
ToastLink.module.scssreferenced a nonexistent--fidesui-font-size-mdCSS varCustomTooltip.module.scssstill used Sassmap.getinstead of CSS varsFollow-up work to migrate all CSS variable generation into Ant Design's token system (eliminating the Sass/TS duplication) is tracked in ENG-3479.
Code Changes
clients/fidesui/src/palette/palette.tswith all hex values as a typed TS objectdefault-theme.ts,dark-theme.ts,FidesUITheme.tsx,palette.stories.tsxto import from the TS module:exportblock frompalette.module.scss(Sass$colorsmap stays for CSS var generation inglobal.scss)--webpackflags from all admin-ui scripts inpackage.jsonToastLink.module.scssto usevar(--ant-font-size)instead of nonexistentvar(--fidesui-font-size-md)CustomTooltip.module.scssfrom Sassmap.getto CSS varSteps to Confirm
cd clients && npm run buildpasses (now uses Turbopack)cd clients/admin-ui && npm run devstarts and colors render correctlycd clients/fidesui && npm run storybookPre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works