feat: handle Automating the Design System with Zero Height #199
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces a comprehensive design system implementation with automated build processes, color palette generation, and styling configurations. The changes span multiple files and packages, focusing on creating a robust, scalable design system using Bun, TypeScript, SCSS, and Tailwind CSS. The implementation includes scripts for generating color tokens, building styles, and configuring Tailwind, with a new GitHub Actions workflow to automate the build process. Changes
Assessment against linked issues
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (15)
packages/design-system/src/styles/base/_colors-light.scss (2)
34-34: Consider using palette variables for consistency.The table header opacity is defined using a direct rgba value. Consider defining this in the palette and referencing it for consistency with other color definitions.
- --table-header: rgba(244, 244, 245, 0.5000); + --table-header: var(--zinc-100-50);
7-7: Standardize the use of color variables.Some properties use
--General-whitewhile others use palette variables. Consider standardizing to use palette variables consistently throughout.- --background: var(--General-white); - --destructive-foreground: var(--General-white); - --primary-foreground: var(--General-white); + --background: var(--zinc-50); + --destructive-foreground: var(--zinc-50); + --primary-foreground: var(--zinc-50);Also applies to: 12-12, 26-26
packages/design-system/src/styles/base/_colors-dark.scss (2)
34-34: Consider using palette variables for consistency.Similar to the light theme, the table header opacity uses a direct rgba value. Consider defining this in the palette for consistency.
- --table-header: rgba(39, 39, 42, 0.5000); + --table-header: var(--zinc-900-50);
6-6: Standardize the use of color variables.Some properties use
--General-whiteor--General-blackwhile others use palette variables. Consider standardizing to use palette variables consistently.- --accent-foreground: var(--General-white); - --card-foreground: var(--General-white); - --destructive-foreground: var(--General-black); + --accent-foreground: var(--zinc-50); + --card-foreground: var(--zinc-50); + --destructive-foreground: var(--zinc-950);Also applies to: 10-10, 12-12
packages/design-system/src/tailwind/config.ts (1)
4-10: Consider adding type definitions for better maintainability.The configuration is clean, but adding TypeScript interfaces would improve type safety and maintainability.
Consider adding these type definitions:
interface ColorConfig { [key: string]: string | Record<string, string>; } interface TailwindConfig { theme: { extend: { colors: ColorConfig; }; }; } export const tailwindConfig: TailwindConfig = { theme: { extend: { colors: { ...colors, ...palette }, }, }, };packages/design-system/src/scripts/utills.ts (1)
1-1: Fix typo in filename: "utills.ts" should be "utils.ts".The filename contains a typo that should be corrected for consistency.
Rename the file from
utills.tstoutils.ts.packages/design-system/src/scripts/build-main-style.ts (1)
11-12: Consider expanding style imports for a complete design system.The main.scss file currently only imports color styles. Consider adding other essential design system components like typography, spacing, layout, and components.
@use "./base/_colors-dark.scss" as *; @use "./base/_colors-light.scss" as *; +@use "./base/_typography.scss" as *; +@use "./base/_spacing.scss" as *; +@use "./base/_layout.scss" as *; +@use "./components/_index.scss" as *;packages/design-system/src/scripts/build-config-tailwind.ts (1)
11-12: Consider using absolute imports for better maintainability.Relative imports can become difficult to maintain as the project grows. Consider using path aliases or absolute imports.
-import { colors } from "./colors"; -import { palette } from "./palette"; +import { colors } from "@repo/design-system/tailwind/colors"; +import { palette } from "@repo/design-system/tailwind/palette";packages/design-system/src/tailwind/colors.ts (1)
1-35: Add TypeScript types and group related colors.The color system would benefit from proper TypeScript types and logical grouping of related colors.
+type ColorToken = string; + +interface ColorSystem { + // Base colors + background: ColorToken; + foreground: ColorToken; + + // Primary colors + primary: ColorToken; + 'primary-foreground': ColorToken; + 'primary-light': ColorToken; + + // Status colors + success: ColorToken; + 'success-border': ColorToken; + 'success-foreground': ColorToken; + // ... other groups +} + -export const colors = { +export const colors: ColorSystem = { // ... existing colors };Consider organizing colors into logical groups with comments for better maintainability.
packages/design-system/src/scripts/build-palette-style.ts (1)
6-7: Consider making file paths configurable.The hardcoded paths relative to
__dirnamemake the script less flexible. Consider accepting these as parameters or environment variables for better configurability.-const inputFilePath = path.join(__dirname, '../tokens/token_Palette_Primitive.json'); -const outputFilePath = path.join(__dirname, '../styles/base/_palette.scss'); +const INPUT_PATH = process.env.PALETTE_INPUT_PATH || '../tokens/token_Palette_Primitive.json'; +const OUTPUT_PATH = process.env.PALETTE_OUTPUT_PATH || '../styles/base/_palette.scss'; +const inputFilePath = path.join(__dirname, INPUT_PATH); +const outputFilePath = path.join(__dirname, OUTPUT_PATH);packages/design-system/src/scripts/build-colors-style.ts (2)
7-9: Improve token reference resolution.The regex pattern for resolving token references could be more robust and maintainable.
+const TOKEN_REFERENCE_PATTERN = /\{([^{}]+)\}/g; + const resolveTokenReference = (value: string): string => { - return value.replace(/{(.+?)}/g, (_, match) => `var(--${match.replace(".", "-")})`); + if (typeof value !== 'string') { + throw new Error(`Invalid token value: ${value}`); + } + return value.replace(TOKEN_REFERENCE_PATTERN, (_, match) => { + const sanitizedMatch = match.trim().replace(".", "-"); + if (!/^[\w-]+$/.test(sanitizedMatch)) { + throw new Error(`Invalid token reference: ${match}`); + } + return `var(--${sanitizedMatch})`; + }); };
12-42: Make theme handling more extensible.The theme handling is currently limited to "dark" and "light". Consider making it more extensible for future theme additions.
+type Theme = { + name: string; + selector: string; + tokenFile: string; +}; + +const THEMES: Record<string, Theme> = { + dark: { + name: 'dark', + selector: ':root', + tokenFile: 'token_Color_Default.json', + }, + light: { + name: 'light', + selector: '.light', + tokenFile: 'token_Color_Light.json', + }, +}; + -const generateScssContent = ( - theme: "dark" | "light", - tokens: Record<string, any>, -): string => { +const generateScssContent = (theme: Theme, tokens: Record<string, any>): string => { const lines: string[] = []; - const selector = theme === "dark" ? ":root" : ".light"; + const { selector } = theme;.github/workflows/build-design-system.yml (2)
26-30: Add caching for dependencies.Implement dependency caching to improve workflow performance and reduce build times.
Add this step before installing dependencies:
+ # Cache dependencies + - name: Cache dependencies + uses: actions/cache@v3 + with: + path: ~/.bun/install/cache + key: ${{ runner.os }}-bun-${{ hashFiles('**/bun.lockb') }} + restore-keys: | + ${{ runner.os }}-bun- + # Step 3: Navigate to the design-system package and install dependencies
31-34: Upload build artifacts.Consider uploading the build output as artifacts for debugging or downstream workflows.
Add this step after the build:
- name: Run build script working-directory: ./packages/design-system run: bun run build + + # Upload build artifacts + - name: Upload build artifacts + uses: actions/upload-artifact@v3 + with: + name: design-system-build + path: packages/design-system/distpackages/ui/package.json (1)
Line range hint
4-9: Add SCSS-related scripts.Consider adding scripts for SCSS compilation and watching during development.
Add these scripts to the
scriptssection:"scripts": { "ui:add": "pnpm dlx shadcn-ui@latest add", "magic:add": "pnpm dlx magicui-cli add", "lint": "eslint .", - "type-check": "bun tsc --noEmit" + "type-check": "bun tsc --noEmit", + "sass:build": "sass src/styles:dist/styles --no-source-map", + "sass:watch": "sass src/styles:dist/styles --watch --no-source-map" },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (30)
.github/workflows/build-design-system.yml(1 hunks).github/workflows/update-leaderboard.yml(0 hunks).github/workflows/updateLeaderboard.js(0 hunks).hintrc(1 hunks)apps/core/app/auth/layout.tsx(1 hunks)apps/core/app/layout.tsx(1 hunks)packages/design-system/package.json(1 hunks)packages/design-system/src/scripts/build-colors-style.ts(1 hunks)packages/design-system/src/scripts/build-colors-tailwind.ts(1 hunks)packages/design-system/src/scripts/build-config-tailwind.ts(1 hunks)packages/design-system/src/scripts/build-main-style.ts(1 hunks)packages/design-system/src/scripts/build-palette-style.ts(1 hunks)packages/design-system/src/scripts/build-palette-tailwind.ts(1 hunks)packages/design-system/src/scripts/build.ts(1 hunks)packages/design-system/src/scripts/utills.ts(1 hunks)packages/design-system/src/styles/base/_colors-dark.scss(1 hunks)packages/design-system/src/styles/base/_colors-light.scss(1 hunks)packages/design-system/src/styles/base/_palette.scss(1 hunks)packages/design-system/src/styles/main.scss(1 hunks)packages/design-system/src/tailwind/colors.ts(1 hunks)packages/design-system/src/tailwind/config.ts(1 hunks)packages/design-system/src/tailwind/palette.ts(1 hunks)packages/design-system/tsconfig.json(1 hunks)packages/design-system/tsconfig.lint.json(1 hunks)packages/ui/package.json(4 hunks)packages/ui/postcss.config.mjs(1 hunks)packages/ui/src/globals.css(0 hunks)packages/ui/src/styles/_tailinwd.scss(1 hunks)packages/ui/src/styles/globals.scss(1 hunks)packages/ui/tailwind.config.ts(3 hunks)
💤 Files with no reviewable changes (3)
- packages/ui/src/globals.css
- .github/workflows/updateLeaderboard.js
- .github/workflows/update-leaderboard.yml
✅ Files skipped from review due to trivial changes (9)
- packages/ui/src/styles/_tailinwd.scss
- apps/core/app/layout.tsx
- apps/core/app/auth/layout.tsx
- packages/design-system/tsconfig.json
- packages/ui/src/styles/globals.scss
- packages/design-system/src/styles/main.scss
- packages/design-system/package.json
- packages/design-system/tsconfig.lint.json
- packages/design-system/src/styles/base/_palette.scss
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/build-design-system.yml
18-18: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 GitHub Actions: Build Design System
packages/design-system/src/scripts/build-palette-tailwind.ts
[error] 36-36: File not found: Unable to read '/home/runner/work/pixel-client/pixel-client/packages/design-system/src/tokens/token_Palette_Primitive.json'. The required input file does not exist in the specified location.
🔇 Additional comments (5)
packages/design-system/src/styles/base/_colors-dark.scss (1)
1-39: LGTM! Well-structured color system implementation.The color system implementation is well-organized with:
- Clear semantic naming
- Consistent variable structure
- Good coverage of UI states
- Appropriate color choices for dark theme
.hintrc (1)
5-7: Document the rationale for disabling consistent-casing and verify its impact.Disabling the TypeScript casing consistency rule could lead to inconsistent naming conventions across the codebase. Please consider:
- Adding a comment explaining why this rule needs to be disabled
- Verifying that this won't conflict with other TypeScript configurations in the design system
Let's check for any existing casing inconsistencies and TypeScript configurations:
✅ Verification successful
Disabling typescript-config/consistent-casing appears safe based on codebase analysis
The codebase demonstrates consistent naming conventions throughout:
- All files follow kebab-case naming
- Exports use standard camelCase
- No casing conflicts detected in the monorepo structure
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for TypeScript configurations and potential casing inconsistencies # Check for other TypeScript config files echo "Searching for TypeScript config files..." fd -e json -e jsonc tsconfig # Check for potential casing inconsistencies in TypeScript/JavaScript files echo "Checking for files with different casing patterns..." fd -e ts -e tsx -e js -e jsx | python3 -c ' import sys import re from collections import defaultdict patterns = defaultdict(list) for line in sys.stdin: filename = line.strip() base = filename.lower() if base: patterns[base].append(filename) for base, variants in patterns.items(): if len(variants) > 1: print(f"\nPotential casing inconsistency found:") for v in variants: print(f" {v}") ' # Look for any existing casing-related lint configurations echo "Checking for other linting configurations..." fd -e json -e js -e yaml -e yml | xargs rg -l "casing|naming"Length of output: 1391
Script:
#!/bin/bash # Check tsconfig files and lint configurations more precisely # Examine tsconfig files for any casing-related configurations echo "=== TypeScript Configurations ===" for config in $(fd -e json -e jsonc tsconfig); do echo "=== $config ===" cat "$config" | grep -A 5 -B 5 "forceConsistentCasingInFileNames\|strict" done # Look for lint configurations more carefully echo -e "\n=== Lint Configurations ===" fd -e json .hintrc .eslintrc fd -e json -e js -e yaml -e yml | rg -l "\.eslintrc|\.hintrc" # Check for documentation about linting decisions echo -e "\n=== Documentation about Linting ===" rg -l -g "!node_modules" -g "!dist" "consistent-casing|naming conventions|lint|hint" "*.md"Length of output: 2230
Script:
#!/bin/bash # Final verification attempt # Check the current .hintrc content echo "=== Current .hintrc Configuration ===" cat .hintrc # Search for documentation with corrected glob patterns echo -e "\n=== Documentation Search ===" rg -l "lint|hint|naming|casing" -g "*.md" -g "!node_modules" -g "!dist" # Analyze casing patterns in code echo -e "\n=== Casing Patterns in Code ===" # Look for different casing styles in exports/declarations ast-grep --pattern 'export const $_ = $_' -l typescript ast-grep --pattern 'export class $_ $_' -l typescriptLength of output: 68021
packages/ui/postcss.config.mjs (1)
3-3: LGTM! Verify PostCSS SCSS syntax plugin installation.The addition of SCSS syntax support aligns with the project's transition to SCSS.
Let's verify the PostCSS SCSS syntax plugin is properly installed:
✅ Verification successful
✅ PostCSS SCSS syntax plugin is properly installed
The
postcss-scssdependency (v4.0.9) is correctly declared in the UI package's dependencies, matching the configuration inpostcss.config.mjs.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if postcss-scss is listed in package dependencies jq '.dependencies["postcss-scss"] // .devDependencies["postcss-scss"]' package.jsonLength of output: 90
Script:
#!/bin/bash # Check if postcss-scss is listed in UI package dependencies cd packages/ui && jq '.dependencies["postcss-scss"] // .devDependencies["postcss-scss"]' package.jsonLength of output: 111
packages/ui/tailwind.config.ts (1)
Line range hint
8-13: Verify content paths for completeness.The content paths look comprehensive but verify if any new component directories need to be included.
✅ Verification successful
Content paths are correctly configured
The current Tailwind content paths cover all necessary files. The additional TypeScript files found in other packages either don't contain UI components (apis, design-system) or use static class names instead of Tailwind classes (icons).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all TypeScript/React files in the repository echo "Files that might need to be included in Tailwind content paths:" fd -e tsx -e ts --type f | grep -vE "(test|spec|stories|mock)" | grep -vE "^(packages/ui|pages|components|app|src)"Length of output: 7439
Script:
#!/bin/bash # Check if icon components use any Tailwind classes echo "Checking for Tailwind class usage in icon components:" rg -l "className" packages/icons/src/components/Length of output: 422
Script:
#!/bin/bash # Check the actual className values in icon components echo "Examining className usage in icon components:" rg "className" packages/icons/src/components/ -C 2Length of output: 2410
packages/design-system/src/tailwind/colors.ts (1)
1-35: Verify CSS custom properties are defined.Ensure that all CSS custom properties used in the color definitions are properly defined in your theme files.
✅ Verification successful
All CSS custom properties are properly defined and managed ✅
The design system has a robust implementation where all CSS custom properties are:
- Defined for both dark (default) and light themes
- Generated from well-documented design tokens
- Properly referenced and layered in the CSS architecture
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Extract CSS custom property names and check their definitions echo "Extracting CSS custom properties from colors.ts..." grep -o -- "--[a-zA-Z-]*" packages/design-system/src/tailwind/colors.ts | sort -u > /tmp/color-vars.txt echo "Checking for undefined CSS custom properties..." while IFS= read -r var; do if ! rg -q "$var" "packages/design-system/src/**/*.scss"; then echo "Warning: $var might be undefined" fi done < /tmp/color-vars.txtLength of output: 19993
Script:
#!/bin/bash # Extract CSS custom property names css_vars=$(grep -o -- "--[a-zA-Z-]*" packages/design-system/src/tailwind/colors.ts | sort -u) echo "Searching for CSS custom property definitions..." for var in $css_vars; do # Search for property definitions (e.g., --primary: ...) echo "Checking $var..." rg -l "\\${var}\\s*:" "packages/design-system/src/**/*.{css,scss}" done # Also check for a global theme file that might contain these definitions echo -e "\nChecking for theme files..." fd -e css -e scss . packages/design-system/src | grep -i "theme\|global\|variables\|tokens"Length of output: 8921
Script:
#!/bin/bash echo "Checking directory structure..." ls -R packages/design-system/src/ echo -e "\nFinding all style-related files..." fd -t f -e css -e scss -e ts -e tsx -e js -e jsx . packages/design-system/src/ echo -e "\nLooking for theme or global style files..." rg -l -g "!node_modules" -t ts -t tsx -t js -t jsx "createTheme|globalStyle|css|styled" packages/design-system/src/Length of output: 2001
Script:
#!/bin/bash echo "Checking color definition files..." cat packages/design-system/src/styles/base/_colors-dark.scss cat packages/design-system/src/styles/base/_colors-light.scss echo -e "\nChecking color tokens..." cat packages/design-system/src/tokens/token_Color_Default.json cat packages/design-system/src/tokens/token_Color_Light.json echo -e "\nChecking build process..." cat packages/design-system/src/scripts/build-colors-style.tsLength of output: 16498
| const main = () => { | ||
| // Styles | ||
| buildPaletteScss(); | ||
| buildColorsScss("light"); | ||
| buildColorsScss("dark"); | ||
| buildMainScss(); | ||
|
|
||
| // Tailiwnd | ||
| buildColorsTailwind(); | ||
| buildPaletteTailwind(); | ||
| buildTailwindConfig(); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and logging for better debugging.
The build script should handle errors gracefully and provide progress feedback.
Consider this improved implementation:
const main = async () => {
try {
console.log('🎨 Building design system...');
// Styles
console.log('Building SCSS styles...');
await buildPaletteScss();
await buildColorsScss("light");
await buildColorsScss("dark");
await buildMainScss();
// Tailwind
console.log('Building Tailwind configurations...');
await buildColorsTailwind();
await buildPaletteTailwind();
await buildTailwindConfig();
console.log('✅ Design system built successfully!');
} catch (error) {
console.error('❌ Build failed:', error);
process.exit(1);
}
};| export const hexToHsl = (hex: string): string => { | ||
| const r = parseInt(hex.slice(1, 3), 16) / 255; | ||
| const g = parseInt(hex.slice(3, 5), 16) / 255; | ||
| const b = parseInt(hex.slice(5, 7), 16) / 255; | ||
|
|
||
| const max = Math.max(r, g, b); | ||
| const min = Math.min(r, g, b); | ||
| const delta = max - min; | ||
|
|
||
| let h = 0; | ||
| if (delta !== 0) { | ||
| if (max === r) { | ||
| h = ((g - b) / delta + (g < b ? 6 : 0)) * 60; | ||
| } else if (max === g) { | ||
| h = ((b - r) / delta + 2) * 60; | ||
| } else { | ||
| h = ((r - g) / delta + 4) * 60; | ||
| } | ||
| } | ||
|
|
||
| const l = (max + min) / 2; | ||
| const s = delta === 0 ? 0 : delta / (1 - Math.abs(2 * l - 1)); | ||
|
|
||
| return `${Math.round(h)} ${Math.round(s * 100)}% ${Math.round(l * 100)}%`; | ||
| }; No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and support for different hex formats.
The current implementation doesn't handle invalid inputs or different hex color formats.
Consider this improved implementation:
export const hexToHsl = (hex: string): string => {
// Validate and normalize hex color
const normalizedHex = hex.toLowerCase().trim();
if (!normalizedHex.startsWith('#')) {
throw new Error('Invalid hex color: must start with #');
}
// Handle different hex formats
let r: number, g: number, b: number;
if (normalizedHex.length === 4) {
// #RGB format
r = parseInt(normalizedHex[1] + normalizedHex[1], 16) / 255;
g = parseInt(normalizedHex[2] + normalizedHex[2], 16) / 255;
b = parseInt(normalizedHex[3] + normalizedHex[3], 16) / 255;
} else if (normalizedHex.length === 7) {
// #RRGGBB format
r = parseInt(normalizedHex.slice(1, 3), 16) / 255;
g = parseInt(normalizedHex.slice(3, 5), 16) / 255;
b = parseInt(normalizedHex.slice(5, 7), 16) / 255;
} else {
throw new Error('Invalid hex color: must be in #RGB or #RRGGBB format');
}
const max = Math.max(r, g, b);
const min = Math.min(r, g, b);
const delta = max - min;
let h = 0;
if (delta !== 0) {
if (max === r) {
h = ((g - b) / delta + (g < b ? 6 : 0)) * 60;
} else if (max === g) {
h = ((b - r) / delta + 2) * 60;
} else {
h = ((r - g) / delta + 4) * 60;
}
}
const l = (max + min) / 2;
const s = delta === 0 ? 0 : delta / (1 - Math.abs(2 * l - 1));
return `${Math.round(h)} ${Math.round(s * 100)}% ${Math.round(l * 100)}%`;
};| fs.writeFileSync(outputFilePath, scssContent, "utf-8"); | ||
| console.log(`main.scss file created at: ${outputFilePath}`); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for file operations.
File operations can fail due to various reasons (permissions, disk space, etc.). Consider adding try-catch blocks for better error handling.
- fs.writeFileSync(outputFilePath, scssContent, "utf-8");
- console.log(`main.scss file created at: ${outputFilePath}`);
+ try {
+ fs.writeFileSync(outputFilePath, scssContent, "utf-8");
+ console.log(`main.scss file created at: ${outputFilePath}`);
+ } catch (error) {
+ console.error(`Failed to create main.scss file: ${error.message}`);
+ process.exit(1);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fs.writeFileSync(outputFilePath, scssContent, "utf-8"); | |
| console.log(`main.scss file created at: ${outputFilePath}`); | |
| try { | |
| fs.writeFileSync(outputFilePath, scssContent, "utf-8"); | |
| console.log(`main.scss file created at: ${outputFilePath}`); | |
| } catch (error) { | |
| console.error(`Failed to create main.scss file: ${error.message}`); | |
| process.exit(1); | |
| } |
| // Write the content to the file | ||
| fs.writeFileSync(outputFilePath, configContent, "utf-8"); | ||
| console.log(`tailwindConfig.ts file created at: ${outputFilePath}`); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for file operations.
Similar to build-main-style.ts, add error handling for file operations.
- fs.writeFileSync(outputFilePath, configContent, "utf-8");
- console.log(`tailwindConfig.ts file created at: ${outputFilePath}`);
+ try {
+ fs.writeFileSync(outputFilePath, configContent, "utf-8");
+ console.log(`tailwindConfig.ts file created at: ${outputFilePath}`);
+ } catch (error) {
+ console.error(`Failed to create tailwindConfig.ts file: ${error.message}`);
+ process.exit(1);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Write the content to the file | |
| fs.writeFileSync(outputFilePath, configContent, "utf-8"); | |
| console.log(`tailwindConfig.ts file created at: ${outputFilePath}`); | |
| // Write the content to the file | |
| try { | |
| fs.writeFileSync(outputFilePath, configContent, "utf-8"); | |
| console.log(`tailwindConfig.ts file created at: ${outputFilePath}`); | |
| } catch (error) { | |
| console.error(`Failed to create tailwindConfig.ts file: ${error.message}`); | |
| process.exit(1); | |
| } |
| const generateColorsTsContent = (tokens: Record<string, any>): string => { | ||
| const colorTokens = tokens.color; | ||
| const lines: string[] = []; | ||
|
|
||
| // Add the TypeScript export header | ||
| lines.push(`export const colors = {`); | ||
|
|
||
| // Process each token and use the key for the variable name | ||
| for (const key in colorTokens) { | ||
| lines.push(` "${key}": "${resolveTokenReference(key)}",`); | ||
| } | ||
|
|
||
| // Close the object | ||
| lines.push(`};`); | ||
|
|
||
| return lines.join("\n"); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type safety and validation to color token processing.
The function lacks type definitions and validation for the color token structure.
+interface ColorValue {
+ $value: string;
+}
+
+interface ColorTokens {
+ color: {
+ [key: string]: ColorValue;
+ };
+}
+
-const generateColorsTsContent = (tokens: Record<string, any>): string => {
+const generateColorsTsContent = (tokens: ColorTokens): string => {
const colorTokens = tokens.color;
+ if (!colorTokens || typeof colorTokens !== 'object') {
+ throw new Error('Invalid token structure: Missing or invalid color tokens');
+ }
const lines: string[] = [];
// Add the TypeScript export header
lines.push(`export const colors = {`);
// Process each token and use the key for the variable name
for (const key in colorTokens) {
+ const token = colorTokens[key];
+ if (!token || typeof token.$value !== 'string') {
+ throw new Error(`Invalid color token: ${key}`);
+ }
lines.push(` "${key}": "${resolveTokenReference(key)}",`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const generateColorsTsContent = (tokens: Record<string, any>): string => { | |
| const colorTokens = tokens.color; | |
| const lines: string[] = []; | |
| // Add the TypeScript export header | |
| lines.push(`export const colors = {`); | |
| // Process each token and use the key for the variable name | |
| for (const key in colorTokens) { | |
| lines.push(` "${key}": "${resolveTokenReference(key)}",`); | |
| } | |
| // Close the object | |
| lines.push(`};`); | |
| return lines.join("\n"); | |
| }; | |
| interface ColorValue { | |
| $value: string; | |
| } | |
| interface ColorTokens { | |
| color: { | |
| [key: string]: ColorValue; | |
| }; | |
| } | |
| const generateColorsTsContent = (tokens: ColorTokens): string => { | |
| const colorTokens = tokens.color; | |
| if (!colorTokens || typeof colorTokens !== 'object') { | |
| throw new Error('Invalid token structure: Missing or invalid color tokens'); | |
| } | |
| const lines: string[] = []; | |
| // Add the TypeScript export header | |
| lines.push(`export const colors = {`); | |
| // Process each token and use the key for the variable name | |
| for (const key in colorTokens) { | |
| const token = colorTokens[key]; | |
| if (!token || typeof token.$value !== 'string') { | |
| throw new Error(`Invalid color token: ${key}`); | |
| } | |
| lines.push(` "${key}": "${resolveTokenReference(key)}",`); | |
| } | |
| // Close the object | |
| lines.push(`};`); | |
| return lines.join("\n"); | |
| }; |
| export const buildColorsScss = (theme: "dark" | "light"): void => { | ||
| // Define the input and output file paths | ||
| const partFileName = theme === "dark" ? "Default" : "Light"; | ||
| const inputFilePath = path.join( | ||
| tokensDir, | ||
| `token_Color_${partFileName}.json`, | ||
| ); | ||
| const outputFilePath = path.join(stylesDir, `_colors-${theme}.scss`); | ||
|
|
||
| // Ensure the input file exists | ||
| if (!fs.existsSync(inputFilePath)) { | ||
| console.error(`Token file not found: ${inputFilePath}`); | ||
| return; | ||
| } | ||
|
|
||
| // Read and parse the JSON file | ||
| const rawData = fs.readFileSync(inputFilePath, "utf-8"); | ||
| const tokens = JSON.parse(rawData); | ||
|
|
||
| // Generate SCSS content | ||
| const scssContent = generateScssContent(theme, tokens); | ||
|
|
||
| // Ensure the output directory exists | ||
| if (!fs.existsSync(stylesDir)) { | ||
| fs.mkdirSync(stylesDir, { recursive: true }); | ||
| } | ||
|
|
||
| // Write the SCSS content to the file | ||
| fs.writeFileSync(outputFilePath, scssContent, "utf-8"); | ||
| console.log(`SCSS file created for ${theme} theme at: ${outputFilePath}`); | ||
| }; |
There was a problem hiding this comment.
Add comprehensive error handling and validation.
The build function needs more robust error handling and validation.
-export const buildColorsScss = (theme: "dark" | "light"): void => {
+export const buildColorsScss = (themeName: string): void => {
+ const theme = THEMES[themeName];
+ if (!theme) {
+ throw new Error(`Invalid theme: ${themeName}`);
+ }
+
// Define the input and output file paths
- const partFileName = theme === "dark" ? "Default" : "Light";
const inputFilePath = path.join(
tokensDir,
- `token_Color_${partFileName}.json`,
+ theme.tokenFile,
);
- const outputFilePath = path.join(stylesDir, `_colors-${theme}.scss`);
+ const outputFilePath = path.join(stylesDir, `_colors-${theme.name}.scss`);
+ try {
// Ensure the input file exists
if (!fs.existsSync(inputFilePath)) {
- console.error(`Token file not found: ${inputFilePath}`);
- return;
+ throw new Error(`Token file not found: ${inputFilePath}`);
}
// Read and parse the JSON file
const rawData = fs.readFileSync(inputFilePath, "utf-8");
const tokens = JSON.parse(rawData);
+ // Validate token structure
+ if (!tokens?.color || typeof tokens.color !== 'object') {
+ throw new Error('Invalid token structure: Missing or invalid color tokens');
+ }
// Generate SCSS content
- const scssContent = generateScssContent(theme, tokens);
+ const scssContent = generateScssContent(theme, tokens);
// Ensure the output directory exists
if (!fs.existsSync(stylesDir)) {
fs.mkdirSync(stylesDir, { recursive: true });
}
// Write the SCSS content to the file
fs.writeFileSync(outputFilePath, scssContent, "utf-8");
console.log(`SCSS file created for ${theme.name} theme at: ${outputFilePath}`);
+ } catch (error) {
+ console.error(`Error building colors SCSS for ${theme.name}:`, error);
+ process.exit(1);
+ }
};Committable suggestion skipped: line range outside the PR's diff.
| "purple": { | ||
| "50": "var(--purple-50)", | ||
| "100": "var(--purple-100)", | ||
| "200": "var(--purple-200)", | ||
| "300": "var(--purple-300)", | ||
| "400": "var(--purple-400)", | ||
| "500": "var(--purple-500)", | ||
| "600": "var(--purple-600)", | ||
| "700": "var(--purple-700)", | ||
| "800": "var(--purple-800)", | ||
| "900": "var(--purple-900)", | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add missing shade to purple palette.
The purple category is missing the "950" shade that exists in all other color categories.
Apply this diff to maintain consistency:
"purple": {
"50": "var(--purple-50)",
// ... other shades ...
"900": "var(--purple-900)",
+ "950": "var(--purple-950)"
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "purple": { | |
| "50": "var(--purple-50)", | |
| "100": "var(--purple-100)", | |
| "200": "var(--purple-200)", | |
| "300": "var(--purple-300)", | |
| "400": "var(--purple-400)", | |
| "500": "var(--purple-500)", | |
| "600": "var(--purple-600)", | |
| "700": "var(--purple-700)", | |
| "800": "var(--purple-800)", | |
| "900": "var(--purple-900)", | |
| }, | |
| "purple": { | |
| "50": "var(--purple-50)", | |
| "100": "var(--purple-100)", | |
| "200": "var(--purple-200)", | |
| "300": "var(--purple-300)", | |
| "400": "var(--purple-400)", | |
| "500": "var(--purple-500)", | |
| "600": "var(--purple-600)", | |
| "700": "var(--purple-700)", | |
| "800": "var(--purple-800)", | |
| "900": "var(--purple-900)", | |
| "950": "var(--purple-950)" | |
| }, |
| "gray": { | ||
| "100": "var(--gray-100)", | ||
| "200": "var(--gray-200)", | ||
| "300": "var(--gray-300)", | ||
| "400": "var(--gray-400)", | ||
| "500": "var(--gray-500)", | ||
| "600": "var(--gray-600)", | ||
| "700": "var(--gray-700)", | ||
| "800": "var(--gray-800)", | ||
| "900": "var(--gray-900)", | ||
| "950": "var(--gray-950)", | ||
| "ali": "var(--gray-ali)", | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Maintain consistency in the gray color palette.
The gray category has inconsistencies:
- Missing the "50" shade that exists in other color categories
- Contains a custom "ali" shade that breaks the numerical pattern
Apply this diff to maintain consistency:
"gray": {
+ "50": "var(--gray-50)",
"100": "var(--gray-100)",
// ... other shades ...
"900": "var(--gray-900)",
- "950": "var(--gray-950)",
- "ali": "var(--gray-ali)",
+ "950": "var(--gray-950)"
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "gray": { | |
| "100": "var(--gray-100)", | |
| "200": "var(--gray-200)", | |
| "300": "var(--gray-300)", | |
| "400": "var(--gray-400)", | |
| "500": "var(--gray-500)", | |
| "600": "var(--gray-600)", | |
| "700": "var(--gray-700)", | |
| "800": "var(--gray-800)", | |
| "900": "var(--gray-900)", | |
| "950": "var(--gray-950)", | |
| "ali": "var(--gray-ali)", | |
| }, | |
| "gray": { | |
| "50": "var(--gray-50)", | |
| "100": "var(--gray-100)", | |
| "200": "var(--gray-200)", | |
| "300": "var(--gray-300)", | |
| "400": "var(--gray-400)", | |
| "500": "var(--gray-500)", | |
| "600": "var(--gray-600)", | |
| "700": "var(--gray-700)", | |
| "800": "var(--gray-800)", | |
| "900": "var(--gray-900)", | |
| "950": "var(--gray-950)" | |
| }, |
| - name: Checkout repository | ||
| uses: actions/checkout@v3 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update the checkout action version.
The current version of actions/checkout@v3 is outdated. Update to the latest version.
Apply this diff:
- name: Checkout repository
- uses: actions/checkout@v3
+ uses: actions/checkout@v4📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Checkout repository | |
| uses: actions/checkout@v3 | |
| - name: Checkout repository | |
| uses: actions/checkout@v4 |
🧰 Tools
🪛 actionlint (1.7.4)
18-18: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
* feat: improve script DS * feat: some chage in json
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/design-system/src/scripts/build.ts (1)
8-30:⚠️ Potential issueHandle asynchronous operations properly.
The current implementation has potential issues with asynchronous operations:
- The function should be async to properly handle promise-based build operations
- Build function calls should be awaited to ensure sequential execution
- Error handling might miss promise rejections without proper await statements
Apply this improved implementation:
-const main = () => { +const main = async () => { try { console.log("🎨 Building design system..."); // Styles console.log("Building SCSS styles..."); - buildPaletteScss(); - buildColorsScss("light"); - buildColorsScss("dark"); - buildMainScss(); + await buildPaletteScss(); + await buildColorsScss("light"); + await buildColorsScss("dark"); + await buildMainScss(); // Tailwind console.log("Building Tailwind configurations..."); - buildColorsTailwind(); - buildPaletteTailwind(); - buildTailwindConfig(); + await buildColorsTailwind(); + await buildPaletteTailwind(); + await buildTailwindConfig(); console.log("✅ Design system built successfully!"); } catch (error) { console.error("❌ Build failed:", error); process.exit(1); } };
🧹 Nitpick comments (1)
packages/design-system/src/scripts/build.ts (1)
1-32: Add TypeScript types for better maintainability.Consider adding type information to improve code maintainability and catch potential issues at compile time:
- Add return type annotations for imported functions
- Add proper error typing
- Add result validation
Example implementation:
// Add types for build functions type BuildResult = { success: boolean; errors?: string[]; }; // Add return type annotations import { buildColorsTailwind }: () => Promise<BuildResult> from "./build-colors-tailwind.js"; // ... similar for other imports const main = async (): Promise<void> => { try { console.log("🎨 Building design system..."); // Styles console.log("Building SCSS styles..."); const paletteResult = await buildPaletteScss(); if (!paletteResult.success) { throw new Error(`Palette SCSS build failed: ${paletteResult.errors?.join(", ")}`); } // ... similar validation for other build results console.log("✅ Design system built successfully!"); } catch (error: unknown) { console.error("❌ Build failed:", error instanceof Error ? error.message : String(error)); process.exit(1); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/design-system/src/scripts/build.ts(1 hunks)packages/design-system/src/tokens/token_Palette_primitive.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/design-system/src/tokens/token_Palette_primitive.json
🔇 Additional comments (1)
packages/design-system/src/scripts/build.ts (1)
1-6: LGTM! Well-organized imports.The imports are logically grouped by functionality and follow a consistent naming pattern.
| } | ||
| }; | ||
|
|
||
| main(); |
There was a problem hiding this comment.
Handle promise rejection for main function execution.
The main function execution should handle potential promise rejections, especially after making it async.
Apply this improvement:
-main();
+main().catch((error) => {
+ console.error("❌ Fatal error:", error);
+ process.exit(1);
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| main(); | |
| main().catch((error) => { | |
| console.error("❌ Fatal error:", error); | |
| process.exit(1); | |
| }); |
Closes #198
Summary by CodeRabbit
Based on the comprehensive summary of changes, here are the updated release notes:
New Features
Styling Updates
Configuration Changes
Workflow Changes
Dependency Updates