feat(DSYS-489): Migrate Text to ADR-0003 and ADR-0004#1047
feat(DSYS-489): Migrate Text to ADR-0003 and ADR-0004#1047georgewrmarshall merged 20 commits intomainfrom
Conversation
📖 Storybook Preview |
📖 Storybook Preview |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
📖 Storybook Preview |
georgewrmarshall
force-pushed
the
cursor/enum-shared-type-migration-1f3a
branch
from
b23ac6d to
25b28ad
Compare
📖 Storybook Preview |
📖 Storybook Preview |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ebc9dd5. Configure here.
packages/design-system-react-native/src/components/HeaderRoot/HeaderRoot.stories.tsx
Outdated
Show resolved
Hide resolved
georgewrmarshall
force-pushed
the
cursor/enum-shared-type-migration-1f3a
branch
from
116713a to
dff9dce
Compare
📖 Storybook Preview |
|
@metamaskbot publish-preview |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
📖 Storybook Preview |
packages/design-system-react-native/src/components/Text/Text.types.ts
Outdated
Show resolved
Hide resolved
📖 Storybook Preview |
|
@metamaskbot publish-preview |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
| */ | ||
| export const FontWeight = { | ||
| /** Weight - 600 */ | ||
| Bold: 'bold', |
There was a problem hiding this comment.
Why semantic values instead of platform values?
FontWeight uses semantic strings ('bold'/'medium'/'regular') rather than Tailwind classes ('font-bold') or React Native numeric weights ('600'). This keeps the shared contract platform-neutral — each platform maps these to its own representation via constants (e.g. MAP_FONTWEIGHT_CLASS in React, TWCLASSMAP_TEXT_FONTWEIGHT in React Native). The same applies to FontFamily and FontStyle.
| * | ||
| * @platform web — CSS cascade has no equivalent in React Native; fails silently on mobile. | ||
| */ | ||
| Inherit: 'text-inherit', |
There was a problem hiding this comment.
Why is Inherit in shared if it's web-only?
text-inherit is a CSS concept with no React Native equivalent — it fails silently on mobile. We chose to keep it in shared rather than create a web-only extension type (TextColorWeb) because that would cause React's TextColor to diverge from shared's, undermining ADR-0004's single-source-of-truth intent. The @platform web JSDoc communicates the limitation without the architectural overhead. Confirmed via manual testing: silent failure, no crash.
| TextVariant, | ||
| } from '@metamask/design-system-shared'; | ||
|
|
||
| export const MAP_FONTWEIGHT_CLASS: Record<FontWeight, string> = { |
There was a problem hiding this comment.
Why are these mapping constants needed?
The shared FontWeight const uses semantic values ('bold'/'medium'/'regular') that can't be passed directly to twMerge — Tailwind expects utility class strings like 'font-bold'. These maps translate the shared semantic contract to Tailwind classes at the React layer. The React Native equivalent (TWCLASSMAP_TEXT_FONTWEIGHT) maps to twrnc font-name suffixes instead.
There was a problem hiding this comment.
Still a valid comment just updated the const to follow TWCLASSMAP prefix
| fontStyle, | ||
| fontFamily, | ||
| fontWeight | ||
| ? MAP_FONTWEIGHT_CLASS[fontWeight] |
There was a problem hiding this comment.
Why the conditional pattern for fontWeight but not fontFamily?
fontWeight is optional and falls back to the variant's default weight (CLASSMAP_TEXT_VARIANT_FONTWEIGHT[variant]) when not provided, so it needs a conditional. fontFamily always has a default value (FontFamily.Default) applied at the component level, so it's always defined here and the map can be called unconditionally.
There was a problem hiding this comment.
Still a valid comment just updated the const to follow TWCLASSMAP prefix
| .fontWeight as FontWeight, | ||
| [TextVariant.AmountDisplayLg]: typography.sAmountDisplayLg | ||
| .fontWeight as FontWeight, | ||
| [TextVariant.DisplayLg]: FontWeight.Bold, |
There was a problem hiding this comment.
Why replace @metamask/design-tokens lookups with explicit values?
Previously, MAP_TEXT_VARIANT_FONTWEIGHT derived values by casting from the typography token object (e.g. typography.sDisplayLG.fontWeight as FontWeight), which created a runtime dependency on the tokens package and required unsafe type casts. The shared FontWeight const now uses semantic strings that don't match the numeric values in @metamask/design-tokens ('600'/'500'/'400'), making the cast invalid. Explicit values are clearer, type-safe, and remove the dependency.
| module.exports = { | ||
| presets: [designSystemPreset], | ||
| content: [ | ||
| '../../packages/design-system-shared/src/**/*.{js,jsx,ts,tsx}', |
There was a problem hiding this comment.
Why does design-system-shared need to be in the content paths?
Before this migration, TextColor and other const values were defined as literal strings inside design-system-react/src/types/index.ts, which was already scanned. After moving to design-system-shared, Tailwind JIT can no longer find those literals during scanning, so utility classes like text-primary-default-hover were never generated — causing hover/pressed text colors to silently render as unstyled. Adding the shared package to content paths restores scanning of the literal strings.
| export type TextProps = { | ||
| import type { OverflowWrap, TextAlign, TextTransform } from '../../types'; | ||
|
|
||
| export type TextProps = TextPropsShared & { |
There was a problem hiding this comment.
Why are TextAlign, TextTransform, OverflowWrap still imported from ../../types while FontWeight/FontFamily/FontStyle moved to shared?
TextAlign, TextTransform, and OverflowWrap are React-only props (web CSS concepts) with no equivalent in the React Native TextProps. They belong in the platform extension layer. FontWeight, FontFamily, and FontStyle apply identically on both platforms and have been promoted to TextPropsShared accordingly.
There was a problem hiding this comment.
Will do a follow up PR to add this functionality to React Native Text out of scope of this PR
Temporarily swaps @metamask/design-system-react, design-system-shared, design-system-tailwind-preset, and design-tokens to their @metamask-previews builds to test MetaMask/metamask-design-system#1047.
…-design-system#1047) Temporarily updates @metamask/design-system-react-native, @metamask/design-system-shared, @metamask/design-system-twrnc-preset, and @metamask/design-tokens to their @metamask-previews builds in order to test the Text component changes in MetaMask/metamask-design-system#1047. Do not merge. This branch is for testing purposes only.
…ared These props are identical across both platforms and belong in the shared type definition rather than being duplicated in each platform package.
georgewrmarshall
force-pushed
the
cursor/enum-shared-type-migration-1f3a
branch
from
2a31304 to
0d90f12
Compare
📖 Storybook Preview |
📖 Storybook Preview |
| | Shared const/type (`TextVariant`, `TextColor`, `FontWeight`, etc.) | `@metamask/design-system-shared` | | ||
| | A sibling component to render it | `'../ComponentName'` | | ||
| | A sibling component's platform-specific props type | `'../ComponentName'` | | ||
| | A sibling component's mapping constants | `'../ComponentName/ComponentName.constants'` | |
There was a problem hiding this comment.
Why can't you import TextVariant from '../Text'?
Both Input and Text are consumers of TextVariant — neither owns it. Importing through '../Text' creates a false semantic coupling (implying Input is built on top of Text), a latent circular-dependency risk (Input → Text → Input if Text ever renders an Input), and obscures that the real source is @metamask/design-system-shared. The table here codifies the rule into a quick reference that scales to every shared type, not just TextVariant.
| * Optional prop to add twrnc overriding classNames. | ||
| */ | ||
| twClassName?: string; | ||
| }; |
There was a problem hiding this comment.
Why is twClassName the only prop left in TextProps?
Everything else — variant, color, fontWeight, fontFamily, fontStyle, children, style — is now covered by TextPropsShared (from shared) intersected with RNTextProps (from react-native). The only genuinely React Native-specific addition is twClassName, which controls TWRNC class overrides and has no web equivalent. This 52 → 13 line reduction is the ADR-0004 payoff: platform-agnostic prop documentation lives once in the shared package, and platform files declare only what's unique to their surface.
| @@ -271,116 +271,6 @@ export enum ButtonIconVariant { | |||
| Floating = 'floating', | |||
There was a problem hiding this comment.
Why delete 116 lines instead of just deprecating them?
These enums (TextVariant, TextColor, FontWeight, FontStyle, FontFamily) were internal implementation details never meant to be imported directly from ../../types by consumers — they were only there because the shared package didn't exist yet. Now that the authoritative definitions live in @metamask/design-system-shared (with const objects per ADR-0003), keeping a second copy here would mean two sources of truth diverging silently. Hard deletion forces all import sites to point at the canonical source, which is the right behaviour for a migration that's meant to be complete.
| export type TextTransform = (typeof TextTransform)[keyof typeof TextTransform]; | ||
|
|
||
| /** | ||
| * TextButton - size |
There was a problem hiding this comment.
Why does the React types/index.ts diff look different from the RN one?
React had 155 lines removed vs 116 in RN because it carried extra hover/pressed pseudo-state variants (PrimaryDefaultHover, ErrorDefaultHover, etc.) that RN never had — hover states are a web-only interaction model. Those values now live in TextColor in @metamask/design-system-shared with @note React Native: Not applicable JSDoc, making the platform distinction explicit in the shared source rather than implicit in platform-specific files.
The three web-only const objects that remain here (TextAlign, OverflowWrap, TextTransform) have been converted from enum to ADR-0003 const objects — they're web-only layout utilities with no RN equivalent, so they stay in this file rather than moving to shared.
📖 Storybook Preview |
Moves the full IconColor const object (union of all React and React Native values) into @metamask/design-system-shared following the same pattern as TextColor in PR #1047. Both platforms now re-export IconColor directly from shared — the platform-local definition and the IconColorBase alias are removed.
📖 Storybook Preview |
Moves the full IconColor const object (union of all React and React Native values) into @metamask/design-system-shared following the same pattern as TextColor in PR #1047. Both platforms now re-export IconColor directly from shared — the platform-local definition and the IconColorBase alias are removed.
## Release 32.0.0 This release migrates `Text` typography types to `@metamask/design-system-shared`, continuing the ADR-0003/0004 const-object + string-union pattern adoption. ### 📦 Package Versions - `@metamask/design-system-shared`: **0.10.0** - `@metamask/design-system-react-native`: **0.17.0** ### 🔄 Shared Type Updates (0.10.0) #### Text typography types added ([#1047](#1047)) **What Changed:** - Added `TextVariant`, `TextColor`, and `TextPropsShared` shared types for cross-platform use **Impact:** - Enables consistent `Text` type definitions across React and React Native - Continues ADR-0003/0004 const-object + string-union pattern adoption ### 📱 React Native Updates (0.17.0) #### Changed - **BREAKING:** Migrated `Text` typography types (`TextVariant`, `TextColor`, `FontWeight`, `FontStyle`, `FontFamily`) to `@metamask/design-system-shared`; all imports through `@metamask/design-system-react-native` continue to work without change ([#1047](#1047)) - `FontWeight` underlying string values changed from numeric strings (`'600'`, `'500'`, `'400'`) to semantic identifiers (`'bold'`, `'medium'`, `'regular'`); idiomatic usage (e.g. `FontWeight.Bold`) is unaffected ###⚠️ Breaking Changes #### `FontWeight` string values changed (React Native Only) **What Changed:** - `FontWeight` was previously a TypeScript `enum` with numeric string values; it is now a `const` object with semantic string values | Key | Before (0.16.0) | After (0.17.0) | | -------------------- | --------------- | -------------- | | `FontWeight.Bold` | `'600'` | `'bold'` | | `FontWeight.Medium` | `'500'` | `'medium'` | | `FontWeight.Regular` | `'400'` | `'regular'` | **Migration:** ```tsx // ❌ Rare: comparing against raw numeric string if (fontWeight === '600') { ... } // ✅ Use const member (works in both 0.16.0 and 0.17.0) if (fontWeight === FontWeight.Bold) { ... } ``` **Impact:** - Only affects code that compared against raw `FontWeight` numeric string values directly - Idiomatic usage (`fontWeight={FontWeight.Bold}`) is unaffected See migration guide for complete instructions: - [React Native Migration Guide](./packages/design-system-react-native/MIGRATION.md#from-version-0160-to-0170) ### ✅ Checklist - [x] Changelogs updated with human-readable descriptions - [x] Changelog validation passed (`yarn changelog:validate`) - [x] Version bumps follow semantic versioning - [x] design-system-shared: minor (0.9.0 → 0.10.0) - new Text shared types added - [x] design-system-react-native: minor (0.16.0 → 0.17.0) - breaking FontWeight value change - [x] Breaking changes documented with migration guidance - [x] Migration guides updated with before/after examples - [x] PR references included in changelog entries ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) - [x] I've reviewed the [Release Workflow](./.cursor/rules/release-workflow.md) cursor rule - [x] All tests pass (`yarn build && yarn test && yarn lint`) - [x] Changelog validation passes (`yarn changelog:validate`) ## **Pre-merge reviewer checklist** - [ ] I've reviewed the [Reviewing Release PRs](./docs/reviewing-release-prs.md) guide - [ ] Package versions follow semantic versioning - [ ] Changelog entries are consumer-facing (not commit message regurgitation) - [ ] Breaking changes are documented in MIGRATION.md with examples - [ ] All unreleased changes are accounted for in changelogs <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Medium risk because this is a release bump that documents a *breaking* underlying `FontWeight` runtime string-value change; consumers comparing raw string values may break even though import paths remain stable. > > **Overview** > Bumps the monorepo to `32.0.0` and releases `@metamask/design-system-react-native` `0.17.0` and `@metamask/design-system-shared` `0.10.0`. > > Updates changelogs to reflect moving `Text` typography types (`TextVariant`, `TextColor`, `FontWeight`, `FontStyle`, `FontFamily`, `TextPropsShared`) into `@metamask/design-system-shared`, including a **breaking** note that `FontWeight` raw string values change from numeric strings to semantic identifiers. > > Adds a release-workflow step to run `yarn changelog:validate` after editing changelogs. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit a05e001. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
3 participants
Description
This PR migrates the
Textcomponent to align with ADR-0003 (String Unions) and ADR-0004 (Centralized Types Architecture) as part of epic DSYS-468.Changes
New shared types (
@metamask/design-system-shared)TextVariant— converted from duplicatedenum TextVariantin both platform packages to a singleconstobject with string union type (ADR-0003). Values are identical across React and React Native.TextColor— new sharedconstobject containing all common color values across both platforms (ADR-0003). React extends this with hover states (PrimaryDefaultHover,ErrorDefaultHover,SuccessDefaultHover,WarningDefaultHover) andInherit. React Native extends withPrimaryAlternative.TextPropsShared— new shared props type (ADR-0004) containing cross-platform Text props:variant,children, andcolor(typed asstringto allow platform-specific extensions).Updated platform packages (
design-system-reactanddesign-system-react-native)enum TextVariantfrom bothsrc/types/index.ts— now re-exported from sharedenum TextColorfrom bothsrc/types/index.ts— replaced with platform-extended const objectsenum FontWeight,enum FontFamily,enum FontStylefrom bothsrc/types/index.ts— converted to const objects (platform-specific values preserved)enum TextAlign,enum OverflowWrap,enum TextTransformfrom Reactsrc/types/index.ts— converted to const objects (React/Tailwind-specific)Text.types.tsin both packages now extendsTextPropsSharedfrom sharedText/index.tsin both packages exportsTextVariantandTextPropsSharedfrom sharedPlatform differences preserved
FontWeight: React uses Tailwind classes (font-bold,font-medium,font-regular); RN uses numeric values (600,500,400)FontFamily: React uses Tailwind classes (font-default,font-accent,font-hero); RN uses plain names (default,accent,hero)FontStyle: React uses Tailwind values (italic,not-italic); RN uses CSS values (italic,normal)TextAlign,OverflowWrap,TextTransform: React-only (Tailwind-specific)Related issues
Fixes: DSYS-489
Manual testing steps
yarn build— all packages build successfullyyarn test— all tests pass with 100% coverage on changed componentsTextVariantis importable from@metamask/design-system-sharedTextColoris importable from@metamask/design-system-sharedTextPropsSharedis importable from@metamask/design-system-sharedScreenshots/Recordings
Before
No visual changes.
before.text.720.mov
After
No visual changes. Text component in storybook works as expected and no visual regressions picked up in Chromatic run
text.migration.after.720.mov
Testing preview packages in mobile works with no typescript or visual issues
Test PR: MetaMask/metamask-mobile#28575
Mobile running with MMDS
Textdesign-tokens.pure.black.before720.mov
Testing preview packages in extension works with no typescript or visual issues
Test PR: MetaMask/metamask-extension#41582
Extension running with MMDS
TextExtension storybook running with MMDS
TextPre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Moves
Text-related enums to shared const-object/string-union types and updates both React and React Native implementations to map these semantic values to platform-specific class names, which could cause subtle styling or build regressions if any mapping/import is missed. Also changes Tailwind content scanning requirements by relocating class-name strings into@metamask/design-system-shared.Overview
Migrates
Texttypography primitives (TextVariant,TextColor,FontWeight,FontFamily,FontStyle) to@metamask/design-system-sharedas const objects + string unions, and updates bothdesign-system-reactanddesign-system-react-nativeto import/re-export and to base theirTextPropson the new sharedTextPropsShared.Updates React and RN components/tests/stories (including
Input,BannerBase,TextButton,KeyValueRow/Column) to use the shared types, and refactorsText.constantsto map semantic shared values to Tailwind/RN font class behavior. Adds migration notes and a Cursor architecture rule clarifying direct imports from@metamask/design-system-shared, plus updates Storybook React Tailwindcontentscanning to include shared sources.Reviewed by Cursor Bugbot for commit 0553ad0. Bugbot is set up for automated code reviews on this repo. Configure here.