-
Notifications
You must be signed in to change notification settings - Fork 13
Implement framework-specific useId for SSR-compatible component ID generation #4797
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
Open
Copilot
wants to merge
8
commits into
main
Choose a base branch
from
copilot/fix-3419
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
46c04a2
Initial plan
Copilot 03c304d
Implement framework-specific useId for SSR compatibility
Copilot 9cd2c6d
Add comprehensive testing and documentation for useId integration
Copilot 7cfd974
Merge branch 'main' into copilot/fix-3419
mfranzke a9dfac1
Merge branch 'main' into copilot/fix-3419
mfranzke 160d502
Merge branch 'main' into copilot/fix-3419
mfranzke e81e803
Merge branch 'main' into copilot/fix-3419
mfranzke 4946dac
Merge branch 'main' into copilot/fix-3419
michaelmkraus File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| # Framework-Specific ID Generation | ||
|
|
||
| This document explains how component ID generation works across different frameworks for SSR compatibility. | ||
|
|
||
| ## Overview | ||
|
|
||
| Components in this design system that need to generate unique IDs (such as form components) now use framework-specific approaches to ensure SSR (Server-Side Rendering) compatibility. | ||
|
|
||
| ## Implementation | ||
|
|
||
| ### Before | ||
| All frameworks used a custom `uuid()` function which could cause hydration mismatches in SSR scenarios: | ||
| ```javascript | ||
| const id = `component-${uuid()}`; | ||
| ``` | ||
|
|
||
| ### After | ||
| Framework-specific ID generation is implemented through a post-build script: | ||
|
|
||
| #### React Components | ||
| - Use React's `useId()` hook | ||
| - Import: `import { useId } from "react"` | ||
| - Usage: `const id = \`component-\${useId()}\`` | ||
|
|
||
| #### Vue Components | ||
| - Use Vue's `useId()` hook | ||
| - Import: `import { useId } from "vue"` | ||
| - Usage: `const id = \`component-\${useId()}\`` | ||
|
|
||
| #### Angular & Stencil Components | ||
| - Continue using `uuid()` function (fallback) | ||
| - Usage: `const id = \`component-\${uuid()}\`` | ||
|
|
||
| ## Benefits | ||
|
|
||
| 1. **SSR Compatibility**: React and Vue components now generate consistent IDs between server and client | ||
| 2. **Hydration Safety**: Eliminates hydration mismatches in SSR applications | ||
| 3. **Framework-Appropriate**: Uses each framework's recommended approach for ID generation | ||
| 4. **Backward Compatibility**: Frameworks without native `useId()` support continue to work | ||
|
|
||
| ## Affected Components | ||
|
|
||
| The following components have been updated to use framework-specific ID generation: | ||
| - textarea | ||
| - switch | ||
| - select | ||
| - radio | ||
| - input | ||
| - custom-select-list-item | ||
| - custom-select | ||
| - checkbox | ||
|
|
||
| ## Technical Details | ||
|
|
||
| ### Post-Build Processing | ||
| A post-build script (`scripts/post-build/use-id.ts`) automatically: | ||
| 1. Scans generated React and Vue components | ||
| 2. Adds appropriate `useId` imports | ||
| 3. Replaces `uuid()` calls with `useId()` calls in ID generation patterns | ||
| 4. Leaves Angular and Stencil components unchanged | ||
|
|
||
| ### Pattern Recognition | ||
| The script identifies ID generation patterns like: | ||
| ```javascript | ||
| `component-${uuid()}` | ||
| ``` | ||
|
|
||
| And transforms them to: | ||
| ```javascript | ||
| `component-${useId()}` // React/Vue only | ||
| ``` | ||
|
|
||
| ## Testing | ||
|
|
||
| Run the verification script to ensure proper integration: | ||
| ```bash | ||
| npx tsx scripts/verify-use-id.ts | ||
| ``` | ||
|
|
||
| This verifies: | ||
| - Correct `useId` usage in React/Vue components | ||
| - Proper import statements | ||
| - No remaining `uuid()` patterns in ID generation | ||
| - Angular/Stencil components still use `uuid()` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| import { readFileSync, writeFileSync } from 'fs'; | ||
| import { globSync } from 'glob'; | ||
| import path from 'path'; | ||
|
|
||
| /** | ||
| * Post-build script to replace uuid() calls with framework-specific useId() hooks | ||
| * for SSR compatibility in React and Vue components. | ||
| */ | ||
|
|
||
| const processReactFiles = () => { | ||
| const files = globSync('../../output/react/src/**/*.tsx'); | ||
| console.log(`Found ${files.length} React files to process`); | ||
|
|
||
| files.forEach(filePath => { | ||
| let content = readFileSync(filePath, 'utf8'); | ||
| let modified = false; | ||
|
|
||
| // Check if file uses uuid for ID generation | ||
| if (content.includes('uuid()') && content.includes('-${uuid()}')) { | ||
| console.log(`Processing file: ${filePath}`); | ||
|
|
||
| // Add useId import if not already present | ||
| if (!content.includes('useId') && !content.includes('import { useId }')) { | ||
| console.log('Adding useId import'); | ||
| content = content.replace( | ||
| /import \* as React from "react";/, | ||
|
Contributor
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. This is very specific. What if the import is like
the replacement would not work. Do we have to consider this? |
||
| 'import * as React from "react";\nimport { useId } from "react";' | ||
| ); | ||
| modified = true; | ||
| } | ||
|
|
||
| // Replace uuid() calls with useId() in ID generation patterns | ||
| const beforeReplace = content; | ||
| content = content.replace( | ||
| /`([^`]*)-\$\{uuid\(\)\}`/g, | ||
| '`$1-${useId()}`' | ||
| ); | ||
| if (content !== beforeReplace) { | ||
| console.log('Replaced uuid() with useId()'); | ||
| modified = true; | ||
| } | ||
| } | ||
|
|
||
| if (modified) { | ||
| writeFileSync(filePath, content); | ||
| console.log(`Updated React file: ${path.relative(process.cwd(), filePath)}`); | ||
| } | ||
| }); | ||
| }; | ||
|
|
||
| const processVueFiles = () => { | ||
| const files = globSync('../../output/vue/src/**/*.vue'); | ||
|
|
||
| files.forEach(filePath => { | ||
| let content = readFileSync(filePath, 'utf8'); | ||
| let modified = false; | ||
|
|
||
| // Check if file uses uuid for ID generation | ||
| if (content.includes('uuid()') && content.includes('-${uuid()}')) { | ||
| // Add useId import if not already present | ||
| if (!content.includes('useId')) { | ||
| // Find existing imports and add useId | ||
| content = content.replace( | ||
| /(import.*from.*vue.*)/, | ||
| '$1\nimport { useId } from "vue";' | ||
| ); | ||
| modified = true; | ||
| } | ||
|
|
||
| // Replace uuid() calls with useId() in ID generation patterns | ||
| content = content.replace( | ||
| /`([^`]*)-\$\{uuid\(\)\}`/g, | ||
| '`$1-${useId()}`' | ||
| ); | ||
| modified = true; | ||
| } | ||
|
|
||
| if (modified) { | ||
| writeFileSync(filePath, content); | ||
| console.log(`Updated Vue file: ${path.relative(process.cwd(), filePath)}`); | ||
| } | ||
| }); | ||
| }; | ||
|
|
||
| export default function UseId() { | ||
| // Run the post-processing | ||
| console.log('🔄 Processing React files for useId integration...'); | ||
| try { | ||
| processReactFiles(); | ||
| } catch (error) { | ||
| console.error('Error processing React files:', error); | ||
| } | ||
|
|
||
| console.log('🔄 Processing Vue files for useId integration...'); | ||
| try { | ||
| processVueFiles(); | ||
| } catch (error) { | ||
| console.error('Error processing Vue files:', error); | ||
| } | ||
|
|
||
| console.log('✅ Post-build useId processing completed!'); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| /** | ||
| * Simple verification script to ensure useId integration is working correctly | ||
| */ | ||
|
|
||
| import { readFileSync } from 'fs'; | ||
| import { globSync } from 'glob'; | ||
|
|
||
| console.log('🧪 Verifying useId integration...\n'); | ||
|
|
||
| // Test 1: Verify React components have useId | ||
| console.log('📋 React Components:'); | ||
| const reactFiles = globSync('../../output/react/src/components/**/*.tsx'); | ||
| let reactProcessed = 0; | ||
|
|
||
| reactFiles.forEach(file => { | ||
| const content = readFileSync(file, 'utf8'); | ||
| if (content.includes('-${useId()}')) { | ||
| const componentName = file.split('/').pop()?.replace('.tsx', ''); | ||
| console.log(` ✅ ${componentName} - uses useId()`); | ||
| reactProcessed++; | ||
| } | ||
| }); | ||
|
|
||
| console.log(` 📊 Total React components using useId: ${reactProcessed}\n`); | ||
|
|
||
| // Test 2: Verify Vue components have useId | ||
| console.log('📋 Vue Components:'); | ||
| const vueFiles = globSync('../../output/vue/src/components/**/*.vue'); | ||
| let vueProcessed = 0; | ||
|
|
||
| vueFiles.forEach(file => { | ||
| const content = readFileSync(file, 'utf8'); | ||
| if (content.includes('-${useId()}')) { | ||
| const componentName = file.split('/').pop()?.replace('.vue', ''); | ||
| console.log(` ✅ ${componentName} - uses useId()`); | ||
| vueProcessed++; | ||
| } | ||
| }); | ||
|
|
||
| console.log(` 📊 Total Vue components using useId: ${vueProcessed}\n`); | ||
|
|
||
| // Test 3: Verify no React/Vue components still use uuid for ID generation | ||
| console.log('🔍 Checking for remaining uuid usage in ID patterns...'); | ||
| let issuesFound = 0; | ||
|
|
||
| [...reactFiles, ...vueFiles].forEach(file => { | ||
| const content = readFileSync(file, 'utf8'); | ||
| const hasUuidIdPattern = content.match(/`[^`]*-\${uuid\(\)}`/); | ||
|
|
||
| if (hasUuidIdPattern) { | ||
| console.log(` ⚠️ ${file} still uses uuid() for ID generation`); | ||
| issuesFound++; | ||
| } | ||
| }); | ||
|
|
||
| if (issuesFound === 0) { | ||
| console.log(' ✅ No uuid-based ID patterns found in React/Vue components\n'); | ||
| } else { | ||
| console.log(` ❌ Found ${issuesFound} components still using uuid for IDs\n`); | ||
| } | ||
|
|
||
| // Test 4: Verify imports are correctly added | ||
| console.log('🔍 Checking import statements...'); | ||
| let importIssues = 0; | ||
|
|
||
| reactFiles.forEach(file => { | ||
| const content = readFileSync(file, 'utf8'); | ||
| if (content.includes('useId()') && !content.includes('import { useId } from "react"')) { | ||
| console.log(` ⚠️ ${file} uses useId() but missing import`); | ||
| importIssues++; | ||
| } | ||
| }); | ||
|
|
||
| vueFiles.forEach(file => { | ||
| const content = readFileSync(file, 'utf8'); | ||
| if (content.includes('useId()') && !content.includes('import { useId } from "vue"')) { | ||
| console.log(` ⚠️ ${file} uses useId() but missing import`); | ||
| importIssues++; | ||
| } | ||
| }); | ||
|
|
||
| if (importIssues === 0) { | ||
| console.log(' ✅ All useId imports are correctly added\n'); | ||
| } else { | ||
| console.log(` ❌ Found ${importIssues} missing import statements\n`); | ||
| } | ||
|
|
||
| // Summary | ||
| console.log('📋 Summary:'); | ||
| console.log(`✅ React components processed: ${reactProcessed}`); | ||
| console.log(`✅ Vue components processed: ${vueProcessed}`); | ||
| console.log(`✅ Components with SSR-compatible IDs: ${reactProcessed + vueProcessed}`); | ||
| console.log(`✅ Import issues: ${importIssues}`); | ||
| console.log(`✅ UUID pattern issues: ${issuesFound}`); | ||
|
|
||
| if (reactProcessed > 0 && vueProcessed > 0 && issuesFound === 0 && importIssues === 0) { | ||
| console.log('\n🎉 All tests passed! SSR-compatible ID generation is working correctly.'); | ||
| } else { | ||
| console.log('\n❌ Some issues found. Please review the output above.'); | ||
| process.exit(1); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| import { expect, test } from '@playwright/test'; | ||
| import { readFileSync } from 'fs'; | ||
| import { globSync } from 'glob'; | ||
|
|
||
| /** | ||
| * Test suite to verify that framework-specific useId hooks are properly | ||
| * integrated for SSR compatibility | ||
| */ | ||
|
|
||
| test.describe('Framework-specific useId integration', () => { | ||
| test('React components should use useId instead of uuid for ID generation', () => { | ||
| const reactFiles = globSync('../../output/react/src/components/**/*.tsx'); | ||
|
|
||
| // Check that components with ID generation use useId | ||
| const filesWithIdGeneration = reactFiles.filter(file => { | ||
| const content = readFileSync(file, 'utf8'); | ||
| return content.includes('-${useId()}') || content.includes('-${uuid()}'); | ||
| }); | ||
|
|
||
| expect(filesWithIdGeneration.length).toBeGreaterThan(0); | ||
|
|
||
| // Verify that React components use useId, not uuid | ||
| filesWithIdGeneration.forEach(file => { | ||
| const content = readFileSync(file, 'utf8'); | ||
|
|
||
| if (content.includes('-${')) { | ||
| // Should use useId, not uuid | ||
| expect(content).toContain('useId()'); | ||
| expect(content).toContain('import { useId } from "react"'); | ||
|
|
||
| // Should not use uuid for ID generation patterns | ||
| expect(content).not.toMatch(/`[^`]*-\${uuid\(\)}`/); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| test('Vue components should use useId instead of uuid for ID generation', () => { | ||
| const vueFiles = globSync('../../output/vue/src/components/**/*.vue'); | ||
|
|
||
| // Check that components with ID generation use useId | ||
| const filesWithIdGeneration = vueFiles.filter(file => { | ||
| const content = readFileSync(file, 'utf8'); | ||
| return content.includes('-${useId()}') || content.includes('-${uuid()}'); | ||
| }); | ||
|
|
||
| expect(filesWithIdGeneration.length).toBeGreaterThan(0); | ||
|
|
||
| // Verify that Vue components use useId, not uuid | ||
| filesWithIdGeneration.forEach(file => { | ||
| const content = readFileSync(file, 'utf8'); | ||
|
|
||
| if (content.includes('-${')) { | ||
| // Should use useId, not uuid | ||
| expect(content).toContain('useId()'); | ||
| expect(content).toContain('import { useId } from "vue"'); | ||
|
|
||
| // Should not use uuid for ID generation patterns | ||
| expect(content).not.toMatch(/`[^`]*-\${uuid\(\)}`/); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| test('Angular and Stencil components should still use uuid for ID generation', () => { | ||
| const angularFiles = globSync('../../output/angular/src/**/*.ts'); | ||
| const stencilFiles = globSync('../../output/stencil/src/**/*.tsx'); | ||
|
|
||
| [...angularFiles, ...stencilFiles].forEach(file => { | ||
| const content = readFileSync(file, 'utf8'); | ||
|
|
||
| if (content.includes('-${') && content.includes('uuid')) { | ||
| // Should still use uuid, not useId (since these frameworks don't have useId) | ||
| expect(content).toContain('uuid()'); | ||
| expect(content).not.toContain('useId()'); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| test('Specific components should have correct ID generation patterns', () => { | ||
| // Test textarea component specifically | ||
| const reactTextarea = readFileSync('../../output/react/src/components/textarea/textarea.tsx', 'utf8'); | ||
| expect(reactTextarea).toContain('`textarea-${useId()}`'); | ||
| expect(reactTextarea).toContain('import { useId } from "react"'); | ||
|
|
||
| const vueTextarea = readFileSync('../../output/vue/src/components/textarea/textarea.vue', 'utf8'); | ||
| expect(vueTextarea).toContain('`textarea-${useId()}`'); | ||
| expect(vueTextarea).toContain('import { useId } from "vue"'); | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
For debugging, the detailed console.log is great, but it might inflate the CI logs. We should check, if we need all log output.