-
-
Notifications
You must be signed in to change notification settings - Fork 8
chore: improve tailwind intellisense and linting for react native #783
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1fac47b
ab606f8
c35ebb7
f191c2c
0ea25d7
6012f49
f5edfd4
7308553
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,12 @@ | ||
| const designSystemPreset = require('@metamask/design-system-tailwind-preset'); | ||
| const { | ||
| generateTailwindConfig, | ||
| Theme, | ||
| } = require('@metamask/design-system-twrnc-preset/tailwind.config'); | ||
|
|
||
| /** @type {import('tailwindcss').Config} */ | ||
| module.exports = { | ||
| presets: [designSystemPreset], | ||
| content: [ | ||
| '../../packages/design-system-react-native/src/**/*.{js,jsx,ts,tsx}', | ||
| './stories/**/*.{js,jsx,ts,tsx}', | ||
| '../../packages/design-system-react-native/src/**/*.{js,jsx,ts,tsx}', | ||
| ], | ||
| theme: { | ||
| // Keep essential semantic colors, remove default palette colors. We want to rely on the colors provided by the design system preset | ||
| colors: { | ||
| inherit: 'inherit', | ||
| current: 'currentColor', | ||
| transparent: 'transparent', | ||
| black: '#000000', | ||
| white: '#ffffff', | ||
| }, | ||
| fontSize: {}, // This removes all default Tailwind font sizes. We want to rely on the design system font sizes and enforce use of the Text component | ||
| extend: {}, | ||
| }, | ||
| plugins: [], | ||
| ...generateTailwindConfig(Theme.Light), | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -252,12 +252,10 @@ const config = createConfig([ | |
| sourceType: 'module', | ||
| }, | ||
| }, | ||
| // Tailwind ESLint | ||
| // Tailwind ESLint for React Web | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Separating eslint tailwind plugin settings so we can point to each tailwind config separately |
||
| { | ||
| files: [ | ||
| 'packages/design-tokens/stories/**', | ||
| 'packages/design-system-react-native/src/**', | ||
| 'apps/storybook-react-native/stories/**', | ||
| 'packages/design-system-react/src/**', | ||
| 'apps/storybook-react/stories/**', | ||
| ], | ||
|
|
@@ -275,12 +273,38 @@ const config = createConfig([ | |
| }, | ||
| settings: { | ||
| tailwindcss: { | ||
| callees: ['twMerge', 'twClassName'], | ||
| callees: ['twMerge'], | ||
| config: 'apps/storybook-react/tailwind.config.js', | ||
| classRegex: ['^(class(Name)?|twClassName)$'], | ||
| }, | ||
georgewrmarshall marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }, | ||
| }, | ||
| // Tailwind ESLint for React Native | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding the same rules for react native but taking in to account twrnc and the new tailwind config |
||
| { | ||
| files: [ | ||
| 'packages/design-system-react-native/src/**', | ||
| 'apps/storybook-react-native/stories/**', | ||
| ], | ||
| plugins: { | ||
| tailwindcss: tailwind, | ||
| }, | ||
| rules: { | ||
| 'tailwindcss/classnames-order': 'error', | ||
| 'tailwindcss/enforces-negative-arbitrary-values': 'error', | ||
| 'tailwindcss/enforces-shorthand': 'error', | ||
| 'tailwindcss/no-arbitrary-value': 'off', // There are legitimate reasons to use arbitrary values but we should specifically error on static colors | ||
| 'tailwindcss/no-custom-classname': 'error', | ||
| 'tailwindcss/no-contradicting-classname': 'error', | ||
| 'tailwindcss/no-unnecessary-arbitrary-value': 'error', | ||
| }, | ||
| settings: { | ||
| tailwindcss: { | ||
| callees: ['twClassName', 'tw'], | ||
| config: 'apps/storybook-react-native/tailwind-intellisense.config.js', | ||
| tags: ['tw'], // Enable template literal support for tw`classnames` | ||
| }, | ||
| }, | ||
| }, | ||
| ]); | ||
|
|
||
| export default config; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ import { render } from '@testing-library/react-native'; | |
| import React from 'react'; | ||
|
|
||
| import { AvatarBaseSize, AvatarBaseShape } from '../../types'; | ||
| import { Text, TextColor, TextVariant } from '../Text'; | ||
| import { Text, TextVariant } from '../Text'; | ||
|
|
||
| import { AvatarBase } from './AvatarBase'; | ||
| import { | ||
|
|
@@ -41,7 +41,8 @@ describe('AvatarBase', () => { | |
| ); | ||
| const fallbackText = getByTestId('fb'); | ||
| expect(fallbackText.props.children).toBe(fallback); | ||
| const expectedTextColor = tw`${TextColor.TextMuted}`.color; | ||
| const expectedTextColor = tw`text-muted`.color; | ||
| // eslint-disable-next-line tailwindcss/no-custom-classname | ||
| const expectedFontSize = tw`text-${TextVariant.BodySm}`.fontSize; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should be able to change this to That said, we should avoid constructing Tailwind classnames using partial strings or string literals. It's harder to read, harder to debug, and more error-prone. Wherever possible, we should define the full classname explicitly—this improves readability, makes debugging easier, and helps with linting and IntelliSense support. |
||
| const expectedMargin = tw`mt-1`.marginTop; | ||
| expect(fallbackText.props.style[0].color).toBe(expectedTextColor); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,7 @@ describe('BadgeIcon', () => { | |
| const TestComponent = () => { | ||
| const tw = useTailwind(); | ||
| // Compute expected container style using an empty twClassName. | ||
| const computedExpectedContainer = tw`h-[16px] w-[16px] items-center justify-center rounded-full bg-icon-default`; | ||
| const computedExpectedContainer = tw`size-[16px] items-center justify-center rounded-full bg-icon-default`; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lint fix |
||
| return ( | ||
| <> | ||
| <BadgeIcon iconName={IconName.Add} testID="badge-icon" /> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -276,7 +276,7 @@ describe('Box', () => { | |
| ); | ||
| const box = getByTestId('box'); | ||
| const styles = flattenStyles(box.props.style); | ||
| expect(styles[0]).toStrictEqual(tw`bg-primary-default flex p-4`); | ||
| expect(styles[0]).toStrictEqual(tw`flex bg-primary-default p-4`); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lint fix |
||
| }); | ||
|
|
||
| it('applies all flex props together', () => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,7 +51,7 @@ export const Box = ({ | |
| ...props | ||
| }: BoxProps) => { | ||
| const tw = useTailwind(); | ||
| const twContainerClassNames = ` | ||
| const twContainerClassNames = tw` | ||
| flex | ||
| ${flexDirection ?? ''} | ||
| ${flexWrap ?? ''} | ||
|
|
@@ -75,10 +75,10 @@ export const Box = ({ | |
| ${borderWidth !== undefined ? TWCLASSMAP_BOX_BORDER_WIDTH[borderWidth] : ''} | ||
| ${borderColor ?? ''} | ||
| ${backgroundColor ?? ''} | ||
| ${twClassName}`.trim(); | ||
| ${twClassName}`; | ||
|
|
||
| return ( | ||
| <View style={[tw`${twContainerClassNames}`, style]} {...props}> | ||
| <View style={[twContainerClassNames, style]} {...props}> | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should try to avoid this way of defining tailwind classnames in react native because unless the classnames appear in |
||
| {children} | ||
| </View> | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,7 +80,7 @@ describe('ButtonIcon', () => { | |
| it('forwards style and twClassName', () => { | ||
| const { result } = renderHook(() => useTailwind()); | ||
| const tw = result.current; | ||
| const expected = tw`items-center justify-center ${TWCLASSMAP_BUTTONICON_SIZE_DIMENSION[ButtonIconSize.Md]} text-primary-default rounded-sm bg-transparent opacity-100`; | ||
| const expected = tw`items-center justify-center ${TWCLASSMAP_BUTTONICON_SIZE_DIMENSION[ButtonIconSize.Md]} rounded-sm bg-transparent text-primary-default opacity-100`; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lint fix |
||
|
|
||
| const { getByTestId } = render( | ||
| <ButtonIcon | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -92,7 +92,7 @@ describe('Checkbox', () => { | |
| expect(styles).toStrictEqual( | ||
| expect.arrayContaining([ | ||
| expect.objectContaining( | ||
| tw`flex h-[22px] w-[22px] items-center justify-center rounded border-2 border-error-default bg-default`, | ||
| tw`flex size-[22px] items-center justify-center rounded border-2 border-error-default bg-default`, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lint fix |
||
| ), | ||
| ]), | ||
| ); | ||
|
|
@@ -112,7 +112,7 @@ describe('Checkbox', () => { | |
| expect(styles).toStrictEqual( | ||
| expect.arrayContaining([ | ||
| expect.objectContaining( | ||
| tw`flex h-[22px] w-[22px] items-center justify-center rounded border-2 border-primary-default bg-primary-default`, | ||
| tw`flex size-[22px] items-center justify-center rounded border-2 border-primary-default bg-primary-default`, | ||
| ), | ||
| ]), | ||
| ); | ||
|
|
@@ -200,7 +200,7 @@ describe('Checkbox', () => { | |
| expect(styles).toStrictEqual( | ||
| expect.arrayContaining([ | ||
| expect.objectContaining( | ||
| tw`flex h-[22px] w-[22px] items-center justify-center rounded border-2 border-default bg-default-pressed`, | ||
| tw`flex size-[22px] items-center justify-center rounded border-2 border-default bg-default-pressed`, | ||
| ), | ||
| ]), | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -128,7 +128,7 @@ export const Checkbox = forwardRef<{ toggle: () => void }, CheckboxProps>( | |
| <AnimatedView | ||
| {...checkboxContainerProps} | ||
| style={[ | ||
| tw`${getCheckboxContainerStyle(pressed)} flex h-[22px] w-[22px] items-center justify-center rounded border-2`, | ||
| tw`${getCheckboxContainerStyle(pressed)} flex size-[22px] items-center justify-center rounded border-2`, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lint fix |
||
| { transform: [{ scale: scaleAnim }] }, | ||
| ]} | ||
| > | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -73,6 +73,7 @@ describe('TextButton', () => { | |
|
|
||
| it('computes baselineOffset correctly for BodyMd', () => { | ||
| const variant = MAP_TEXTBUTTON_SIZE_TEXTVARIANT[TextButtonSize.BodyMd]; | ||
| // eslint-disable-next-line tailwindcss/no-custom-classname | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Disabling no custom rule for this dynamic usage. We should try to avoid this if possible. |
||
| const twStyles = tw`text-${variant}` as { | ||
| fontSize?: number; | ||
| lineHeight?: number; | ||
|
|
@@ -101,6 +102,7 @@ describe('TextButton', () => { | |
| ); | ||
| const txt = innerText({ getAllByText }, sz); | ||
| const variant = MAP_TEXTBUTTON_SIZE_TEXTVARIANT[sz as TextButtonSize]; | ||
| // eslint-disable-next-line tailwindcss/no-custom-classname | ||
| const twStyles = tw`text-${variant}` as { fontSize?: number }; | ||
| const { fontSize = 0 } = twStyles; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,101 @@ or | |
| npm install react@^18.2.0 react-native@0.72.15 twrnc@^4.5.1 | ||
| ``` | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updating docs to provide instructions on setting up tailwind config for intellisense and eslint tailwind plugin |
||
| ## Usage | ||
|
|
||
| ### Using the Theme Provider | ||
|
|
||
| ```tsx | ||
| import { | ||
| ThemeProvider, | ||
| Theme, | ||
| useTailwind, | ||
| } from '@metamask/design-system-twrnc-preset'; | ||
|
|
||
| function App() { | ||
| return ( | ||
| <ThemeProvider theme={Theme.Light}> | ||
| <MyComponent /> | ||
| </ThemeProvider> | ||
| ); | ||
| } | ||
|
|
||
| function MyComponent() { | ||
| const tw = useTailwind(); | ||
|
|
||
| return ( | ||
| <View style={tw`p-4 bg-background-default`}> | ||
| <Text style={tw`text-text-default text-heading-lg`}> | ||
| Hello MetaMask Design System! | ||
| </Text> | ||
| </View> | ||
| ); | ||
| } | ||
| ``` | ||
|
|
||
| ### Tailwind Config for IntelliSense | ||
|
|
||
| To get Tailwind IntelliSense and ESLint plugin support, use the config generator: | ||
|
|
||
| **TypeScript:** | ||
|
|
||
| ```typescript | ||
| // tailwind.config.ts | ||
| import { | ||
| generateTailwindConfig, | ||
| Theme, | ||
| } from '@metamask/design-system-twrnc-preset/tailwind.config'; | ||
|
|
||
| export default { | ||
| content: ['./src/**/*.{js,jsx,ts,tsx}'], | ||
| ...generateTailwindConfig(Theme.Light), | ||
| }; | ||
| ``` | ||
|
|
||
| **JavaScript:** | ||
|
|
||
| ```javascript | ||
| // tailwind.config.js | ||
| const { | ||
| generateTailwindConfig, | ||
| Theme, | ||
| } = require('@metamask/design-system-twrnc-preset/tailwind.config'); | ||
|
|
||
| module.exports = { | ||
| content: ['./src/**/*.{js,jsx,ts,tsx}'], | ||
| ...generateTailwindConfig(Theme.Light), | ||
| }; | ||
| ``` | ||
|
|
||
| **Custom content paths:** | ||
|
|
||
| ```typescript | ||
| // tailwind.config.ts | ||
| import { | ||
| generateTailwindConfig, | ||
| Theme, | ||
| } from '@metamask/design-system-twrnc-preset/tailwind.config'; | ||
|
|
||
| export default { | ||
| content: [ | ||
| './src/**/*.{js,jsx,ts,tsx}', | ||
| './app/**/*.{js,jsx,ts,tsx}', | ||
| './components/**/*.{js,jsx,ts,tsx}', | ||
| './screens/**/*.{js,jsx,ts,tsx}', | ||
| './lib/**/*.{js,jsx,ts,tsx}', | ||
| ], | ||
| ...generateTailwindConfig(Theme.Light), | ||
| }; | ||
| ``` | ||
|
|
||
| This provides: | ||
|
|
||
| - 🎨 **Full IntelliSense support** - Auto-completion for all design system classes | ||
| - 🔍 **ESLint integration** - Works with `eslint-plugin-tailwindcss` | ||
| - 🌙 **Theme agnostic** - Classnames work with both light and dark themes | ||
| - 📝 **Type safety** - TypeScript definitions for all design tokens | ||
| - ⚡ **Actual Design System Config** - Uses the same configuration as the TWRNC preset | ||
|
|
||
| ## Contributing | ||
|
|
||
| This package is part of a monorepo. Instructions for contributing can be found in the [monorepo README](https://github.com/MetaMask/metamask-design-system#readme). | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,16 @@ | |
| "default": "./dist/index.cjs" | ||
| } | ||
| }, | ||
| "./tailwind.config": { | ||
| "import": { | ||
| "types": "./dist/tailwind.config.d.mts", | ||
| "default": "./dist/tailwind.config.mjs" | ||
| }, | ||
| "require": { | ||
| "types": "./dist/tailwind.config.d.cts", | ||
| "default": "./dist/tailwind.config.cjs" | ||
| } | ||
| }, | ||
|
Comment on lines
+30
to
+39
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. JSX vs Node.js: Why We Need the Separate ExportThe Problem:// Main export (packages/design-system-twrnc-preset/src/index.ts)
export { ThemeProvider } from './ThemeProvider'; // ← Contains JSX
export { useTailwind, useTheme } from './hooks'; // ← Contains JSX
export { generateTailwindConfig } from './tailwind.config'; // ← Pure JSWhen compiled, the JSX becomes: // dist/ThemeProvider.cjs
return (<ThemeContext.Provider value={contextValue}> // ← Node.js can't parse this
{children}
</ThemeContext.Provider>);The Issue:
The Solution:// package.json exports
{
".": "./dist/index.cjs", // ← Contains JSX (for React apps)
"./tailwind.config": "./dist/tailwind.config.cjs" // ← No JSX (for Node.js)
}Usage Pattern:// ❌ This crashes in Node.js (ESLint, config files)
const { generateTailwindConfig } = require('@metamask/design-system-twrnc-preset');
// ✅ This works in Node.js
const { generateTailwindConfig } = require('@metamask/design-system-twrnc-preset/tailwind.config');
// ✅ This works in React apps
import { ThemeProvider } from '@metamask/design-system-twrnc-preset';Why This Matters:ESLint needs to load TL;DR: Main export has JSX for React apps, separate export has pure JS for Node.js tools like ESLint. |
||
| "./package.json": "./package.json" | ||
| }, | ||
| "main": "./dist/index.cjs", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,3 +4,6 @@ export { Theme } from './Theme.types'; | |
|
|
||
| // Hooks | ||
| export { useTailwind, useTheme } from './hooks'; | ||
|
|
||
| // Config generation | ||
| export { generateTailwindConfig } from './tailwind.config'; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are exporting |
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactors and streamlines the tailwind-intellisense.config.js used for IntelliSense and the ESLint Tailwind plugin in React Native Storybook stories. Instead of relying on the previous Storybook React Tailwind config, which was similar but not fully compatible, this change imports the canonical Tailwind config directly from @metamask/design-system-twrnc-preset. This ensures the config is accurate, up to date, and fully aligned with the design system used in React Native components, improving both developer experience and consistency across the project.