-
Notifications
You must be signed in to change notification settings - Fork 6
Added Theme builder workflow to detect change in variables #312
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
7692c99
c49d9d6
f7f9c81
734d977
a3bdbce
4264a6f
bc67408
a8a4174
c3314ba
8fd06c9
38c3eb5
7b36bba
cd3e4c1
92d6fb6
6c9b93f
3066ed4
db2c883
dd4367d
fcb85ab
0a02986
860f2af
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 |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| name: CSS Variables Validation | ||
|
|
||
| on: | ||
| pull_request: | ||
| paths: | ||
| - 'src/css-variables.ts' | ||
|
|
||
| jobs: | ||
| validate: | ||
| runs-on: self-hosted | ||
| steps: | ||
| - uses: actions/checkout@v2 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Checkout theme builder repo | ||
| uses: actions/checkout@v2 | ||
| with: | ||
| repository: ${{ secrets.THEME_BUILDER_REPO }} | ||
| path: theme-builder | ||
| ref: release | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Validate CSS variables | ||
| run: | | ||
| node scripts/validate-css-variables.js "$(cat theme-builder/src/data/sdkVariable.ts)" |
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,162 @@ | ||||||||||||||||
| #!/usr/bin/env node | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * Script to validate CSS variables consistency between repositories | ||||||||||||||||
| * This script extracts CSS variable names from the TypeScript interface | ||||||||||||||||
| * and compares them with the implementation in another repository | ||||||||||||||||
| */ | ||||||||||||||||
|
|
||||||||||||||||
| const fs = require('fs'); | ||||||||||||||||
| const path = require('path'); | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * Extract CSS variable names from the TypeScript interface | ||||||||||||||||
| * @param {string} filePath - Path to the css-variables.ts file | ||||||||||||||||
| * @returns {string[]} Array of CSS variable names | ||||||||||||||||
| */ | ||||||||||||||||
| function extractVariablesFromInterface(filePath) { | ||||||||||||||||
| try { | ||||||||||||||||
| const content = fs.readFileSync(filePath, 'utf8'); | ||||||||||||||||
|
|
||||||||||||||||
| // Extract variable names using regex | ||||||||||||||||
| const variableRegex = /'--ts-var-[^']+'/g; | ||||||||||||||||
| const matches = content.match(variableRegex); | ||||||||||||||||
|
|
||||||||||||||||
| if (!matches) { | ||||||||||||||||
| console.error('No CSS variables found in the interface file'); | ||||||||||||||||
| return []; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // Remove quotes and sort for consistent comparison | ||||||||||||||||
| return matches.map(match => match.replace(/'/g, '')).sort(); | ||||||||||||||||
| } catch (error) { | ||||||||||||||||
| console.error(`Error reading interface file: ${error.message}`); | ||||||||||||||||
| return []; | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * Extract CSS variable names from the implementation object | ||||||||||||||||
| * @param {string} content - Content of the implementation file | ||||||||||||||||
| * @returns {string[]} Array of CSS variable names | ||||||||||||||||
| */ | ||||||||||||||||
| function extractVariablesFromImplementation(content) { | ||||||||||||||||
| try { | ||||||||||||||||
| // Extract variable names from object keys | ||||||||||||||||
| const variableRegex = /'--ts-var-[^']+':/g; | ||||||||||||||||
| const matches = content.match(variableRegex); | ||||||||||||||||
|
|
||||||||||||||||
| if (!matches) { | ||||||||||||||||
| console.error('No CSS variables found in the implementation'); | ||||||||||||||||
| return []; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // Remove quotes and colon, then sort for consistent comparison | ||||||||||||||||
| return matches.map(match => match.replace(/[':]/g, '')).sort(); | ||||||||||||||||
| } catch (error) { | ||||||||||||||||
| console.error(`Error parsing implementation: ${error.message}`); | ||||||||||||||||
| return []; | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * Compare two arrays of CSS variables and report differences | ||||||||||||||||
| * @param {string[]} interfaceVars - Variables from TypeScript interface | ||||||||||||||||
| * @param {string[]} implementationVars - Variables from implementation | ||||||||||||||||
| * @returns {object} Comparison result | ||||||||||||||||
| */ | ||||||||||||||||
| function compareVariables(interfaceVars, implementationVars) { | ||||||||||||||||
| const missingInImplementation = interfaceVars.filter(varName => !implementationVars.includes(varName)); | ||||||||||||||||
| const extraInImplementation = implementationVars.filter(varName => !interfaceVars.includes(varName)); | ||||||||||||||||
|
Comment on lines
+69
to
+70
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. For better performance, especially as the number of CSS variables grows, it's more efficient to use You could refactor this section to something like this: const implementationSet = new Set(implementationVars);
const interfaceSet = new Set(interfaceVars);
const missingInImplementation = interfaceVars.filter(varName => !implementationSet.has(varName));
const extraInImplementation = implementationVars.filter(varName => !interfaceSet.has(varName)); |
||||||||||||||||
|
|
||||||||||||||||
| return { | ||||||||||||||||
| interfaceCount: interfaceVars.length, | ||||||||||||||||
| implementationCount: implementationVars.length, | ||||||||||||||||
| missingInImplementation, | ||||||||||||||||
| extraInImplementation, | ||||||||||||||||
| isConsistent: missingInImplementation.length === 0 && extraInImplementation.length === 0 | ||||||||||||||||
| }; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * Main validation function | ||||||||||||||||
| */ | ||||||||||||||||
| function validateCSSVariables() { | ||||||||||||||||
| console.log('🔍 Validating CSS variables consistency...\n'); | ||||||||||||||||
|
|
||||||||||||||||
| // Path to the interface file | ||||||||||||||||
| const interfacePath = path.join(__dirname, '..', 'src', 'css-variables.ts'); | ||||||||||||||||
|
|
||||||||||||||||
| // Check if interface file exists | ||||||||||||||||
| if (!fs.existsSync(interfacePath)) { | ||||||||||||||||
| console.error(`❌ Interface file not found: ${interfacePath}`); | ||||||||||||||||
| process.exit(1); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // Extract variables from interface | ||||||||||||||||
| const interfaceVars = extractVariablesFromInterface(interfacePath); | ||||||||||||||||
| console.log(`📋 Found ${interfaceVars.length} variables in TypeScript interface`); | ||||||||||||||||
|
||||||||||||||||
| const interfaceVars = extractVariablesFromInterface(interfacePath); | |
| console.log(`📋 Found ${interfaceVars.length} variables in TypeScript interface`); | |
| const interfaceVars = extractVariablesFromInterface(interfacePath); | |
| if (interfaceVars === null) { | |
| process.exit(1); | |
| } | |
| console.log(`📋 Found ${interfaceVars.length} variables in TypeScript interface`); |
Outdated
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.
Similar to the interface variable extraction, if extractVariablesFromImplementation fails, the script continues with an empty array. This can be misleading as it would report all interface variables as 'missing'. It's better to handle this failure explicitly and exit. To do this, you should modify extractVariablesFromImplementation to return null on error. Then, you can apply the suggestion below to check for null and exit.
| const implementationVars = extractVariablesFromImplementation(implementationContent); | |
| console.log(`🔧 Found ${implementationVars.length} variables in implementation`); | |
| const implementationVars = extractVariablesFromImplementation(implementationContent); | |
| if (implementationVars === null) { | |
| process.exit(1); | |
| } | |
| console.log(`🔧 Found ${implementationVars.length} variables in implementation`); |
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.
The current logic for extracting variables from the implementation string is a bit brittle. The regular expression on line 46 doesn't account for optional whitespace between the variable name and the colon, which could cause validation to fail on slightly different but valid formatting. The suggestion below adjusts the regex to handle whitespace and updates the mapping function to trim the result, making the extraction more robust.