Security updates, TypeScript improvements, and test coverage #411
Conversation
- Update VitePress to v2.0.0-alpha.12 to fix security vulnerabilities - Replace all TypeScript 'any' types with proper interfaces - Add comprehensive test suites for Alert, Badge, and Card components - Improve TypeScript strict typing across components - Add CLAUDE.md development documentation 🤖 Generated with [Claude Code](https://claude.ai/code)
❌ Deploy Preview for sensational-seahorse-8635f8 failed.
|
WalkthroughAdds Claude local permissions and documentation, tightens TypeScript slot/generic typings across multiple components, relaxes several component prop requirements, updates package version ranges, adds Dependabot and GitHub Actions workflows, and introduces many new component tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Push as Repo Push
participant Actions as GitHub Actions
participant TestJob as test (matrix)
participant SecurityJob as security
participant PublishJob as publish
rect rgb(220,235,255)
Push->>Actions: Trigger workflow
Actions->>TestJob: run tests / lint / typecheck / coverage / build
TestJob-->>Actions: success/failure + coverage artifact
end
rect rgb(235,245,220)
Actions->>SecurityJob: run npm audit & npm outdated
SecurityJob-->>Actions: audit result / outdated.json
end
rect rgb(255,240,220)
Note right of Actions: publish runs only on main and if test+security pass
Actions->>PublishJob: compare local vs npm version
PublishJob->>npm: fetch published version
alt versions differ
PublishJob->>npm: npm publish (uses NODE_AUTH_TOKEN)
else same version
PublishJob-->>Actions: skip publish
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/components/FwbBadge/tests/Badge.spec.ts (2)
24-62: Refactor the type checking logic for better maintainability.The cascading if-else chain is repetitive and violates the DRY principle. Consider using a lookup object to map types to their expected classes.
Apply this diff to refactor the type checking:
it('renders with different types', () => { const types = ['default', 'dark', 'red', 'green', 'yellow', 'indigo', 'purple', 'pink'] as const + const typeClassMap = { + default: { bg: 'bg-blue-100', text: 'text-blue-800' }, + dark: { bg: 'bg-gray-100', text: 'text-gray-800' }, + red: { bg: 'bg-red-100', text: 'text-red-800' }, + green: { bg: 'bg-green-100', text: 'text-green-800' }, + yellow: { bg: 'bg-yellow-100', text: 'text-yellow-800' }, + indigo: { bg: 'bg-indigo-100', text: 'text-indigo-800' }, + purple: { bg: 'bg-purple-100', text: 'text-purple-800' }, + pink: { bg: 'bg-pink-100', text: 'text-pink-800' }, + } types.forEach((type) => { const wrapper = mount(FwbBadge, { props: { type }, slots: { default: `${type} badge` }, }) const classes = wrapper.classes() - - // Check for type-specific classes - if (type === 'default') { - expect(classes).toContain('bg-blue-100') - expect(classes).toContain('text-blue-800') - } else if (type === 'dark') { - expect(classes).toContain('bg-gray-100') - expect(classes).toContain('text-gray-800') - } else if (type === 'red') { - expect(classes).toContain('bg-red-100') - expect(classes).toContain('text-red-800') - } else if (type === 'green') { - expect(classes).toContain('bg-green-100') - expect(classes).toContain('text-green-800') - } else if (type === 'yellow') { - expect(classes).toContain('bg-yellow-100') - expect(classes).toContain('text-yellow-800') - } else if (type === 'indigo') { - expect(classes).toContain('bg-indigo-100') - expect(classes).toContain('text-indigo-800') - } else if (type === 'purple') { - expect(classes).toContain('bg-purple-100') - expect(classes).toContain('text-purple-800') - } else if (type === 'pink') { - expect(classes).toContain('bg-pink-100') - expect(classes).toContain('text-pink-800') - } + const expectedClasses = typeClassMap[type] + expect(classes).toContain(expectedClasses.bg) + expect(classes).toContain(expectedClasses.text) }) })
64-81: Consider refactoring for consistency.For consistency with the suggested refactor for the types test, you could simplify this using a lookup object. However, given the smaller number of cases, the current approach is acceptable.
If you want to maintain consistency:
it('renders with different sizes', () => { const sizes = ['xs', 'sm'] as const + const sizeClassMap = { + xs: 'text-xs', + sm: 'text-sm', + } sizes.forEach((size) => { const wrapper = mount(FwbBadge, { props: { size }, slots: { default: `${size} badge` }, }) const classes = wrapper.classes() - - if (size === 'xs') { - expect(classes).toContain('text-xs') - } else if (size === 'sm') { - expect(classes).toContain('text-sm') - } + expect(classes).toContain(sizeClassMap[size]) }) })src/components/FwbCard/tests/Card.spec.ts (1)
114-122: Consider whether internal structure testing is necessary.This test validates the internal DOM structure (specifically that content is wrapped in a nested
div). While it confirms the current implementation, it couples the test to internal details rather than user-visible behavior or component API.If the wrapping structure is:
- Part of the component's documented contract, or
- Required for proper styling/functionality
then this test is appropriate. Otherwise, consider whether this level of structural testing adds value or simply increases maintenance burden when refactoring.
src/components/FwbJumbotron/FwbJumbotron.vue (1)
49-50: Make the default slot optionalThe jumbotron renders fine without any slot content, but this typing now forces consumers to always provide it. Marking the slot optional keeps the surface area accurate while still documenting that it passes no props.
defineSlots<{ - default: Record<string, never> + default?: Record<string, never> }>()src/components/FwbAutocomplete/tests/Autocomplete.spec.ts (1)
293-305: Leverage the existing option type to avoid castsYou can drop the
Record<string, unknown>+as anyworkaround by reusing the concrete option type thatmockOptionsalready provides.- const displayFn = vi.fn((option: Record<string, unknown>) => `${(option as any).name} (${(option as any).code})`) + const displayFn = vi.fn((option: typeof mockOptions[number]) => `${option.name} (${option.code})`)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
.claude/settings.local.json(1 hunks)CLAUDE.md(1 hunks)package.json(2 hunks)src/components/FwbAlert/FwbAlert.vue(1 hunks)src/components/FwbAlert/tests/Alert.spec.ts(1 hunks)src/components/FwbAutocomplete/FwbAutocomplete.vue(2 hunks)src/components/FwbAutocomplete/tests/Autocomplete.spec.ts(2 hunks)src/components/FwbAutocomplete/types.ts(2 hunks)src/components/FwbBadge/tests/Badge.spec.ts(1 hunks)src/components/FwbCard/tests/Card.spec.ts(1 hunks)src/components/FwbJumbotron/FwbJumbotron.vue(1 hunks)src/components/FwbPagination/FwbPagination.vue(1 hunks)src/components/utils/FwbSlotListener/FwbSlotListener.vue(1 hunks)
🔇 Additional comments (24)
src/components/FwbBadge/tests/Badge.spec.ts (9)
1-5: LGTM!The imports are clean and follow standard vitest and Vue Test Utils conventions.
6-13: LGTM!The default rendering test effectively validates the baseline behavior and uses robust tag name comparison.
15-22: LGTM!The anchor rendering test properly validates the conditional tag switching based on the href prop.
83-93: LGTM!The icon slot test properly validates slot composition with both icon and text content.
95-105: LGTM!The icon-only test effectively validates the conditional padding behavior when no text content is provided.
107-117: LGTM!The base classes test ensures core styling is consistently applied.
119-126: LGTM!The rounded classes test validates important styling behavior.
128-136: LGTM!The padding test validates content-based spacing and complements the icon-only padding test.
138-148: LGTM!The combined icon and text test provides clear validation of both content types appearing together.
src/components/utils/FwbSlotListener/FwbSlotListener.vue (2)
79-124: LGTM!The event handler definitions and merging logic are well-structured. The explicit typing of handlers with
MouseEventandFocusEventensures type safety, and the optional chaining prevents runtime errors when handlers are undefined.
31-35: Retain the explicit cast. TypeScript can’t infer a generic rest‐parameter signature from the specific handler types in TriggerEventHandlers, so casting to (...args: unknown[]) => void is required to safely invoke handler(...args).src/components/FwbCard/tests/Card.spec.ts (10)
7-13: LGTM! Solid baseline rendering test.This test effectively validates the default rendering behavior, confirming that the component renders as a
divand displays slot content correctly.
15-22: LGTM! Proper anchor rendering validation.This test correctly verifies that the component renders as an anchor element when an
hrefprop is provided, and that the attribute is properly set.
24-38: LGTM! Comprehensive image rendering test.This test thoroughly validates image rendering with proper attribute verification and styling class checks.
40-46: LGTM! Proper negative case coverage.This test appropriately validates that no image is rendered when
imgSrcis omitted.
48-81: LGTM! Well-structured parameterized test.This test effectively validates both variants with comprehensive class assertions. The use of
as constprovides good type safety, and the conditional checks appropriately verify variant-specific styling.
83-103: LGTM! Thorough horizontal variant image validation.This test comprehensively verifies all the responsive and styling classes specific to images in the horizontal variant, ensuring proper layout behavior.
105-112: LGTM! Validates custom class application.This test appropriately confirms that custom classes can be applied to the component, which is essential for component customization.
124-133: LGTM! Proper hover state validation.This test appropriately verifies that hover classes are applied when the component renders as an anchor, covering both light and dark mode states.
135-151: LGTM! Validates complex content handling.This test appropriately verifies that the component can handle rich, multi-element slot content while preserving structure and styling.
153-163: Verify accessibility approach for missing alt text.The test confirms
alt=""whenimgAltis omitted andFwbCardhas no validation or warnings. Ensure decorative vs. content image use is documented, consider requiringimgAltor adding a development warning for missing alt text.src/components/FwbAlert/tests/Alert.spec.ts (1)
6-133: Great coverage boostAppreciate how these tests walk through every variant, slot path, and close interaction—the onClose workflow is especially well guarded now.
src/components/FwbAutocomplete/FwbAutocomplete.vue (1)
145-145: LGTM! Type safety improvement.Replacing
anywithunknownin the generic constraint andinputPropstype correctly tightens type safety. Theunknowntype forces consumers to perform explicit type checks before using these values, preventing potential runtime errors.Also applies to: 169-169
src/components/FwbAutocomplete/types.ts (1)
7-7: LGTM! Consistent type safety improvements.The changes from
anytounknowninAutocompleteProps,inputProps, andAutocompleteEmitsalign perfectly with the component updates and improve type safety across the Autocomplete API surface. Theunknowntype requires explicit type narrowing, preventing accidental unsafe operations.Also applies to: 29-29, 33-33
- Fix Vue prop validation warnings in Footer and Sidebar components - Update @types/node to latest version (24.6.2) - Add comprehensive test suites for Spinner, Breadcrumb, Progress, and Toggle components - Increase total test coverage from 60 to 93 tests (+55% increase) - Set up GitHub Actions CI/CD pipeline with automated testing, linting, and builds - Add Dependabot configuration for automated dependency updates - Implement security audit and dependency monitoring workflows 🤖 Generated with [Claude Code](https://claude.ai/code)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (9)
src/components/FwbBreadcrumb/tests/Breadcrumb.spec.ts (2)
85-92: Consider consolidating or enhancing the accessibility test.The aria-label check is already covered in the first test case (line 17). While having a dedicated accessibility test is good practice, you could either:
- Remove this test to avoid duplication, or
- Enhance it to cover additional accessibility concerns (e.g., keyboard navigation, role attributes, or ARIA relationships between breadcrumb items).
94-106: Consider consolidating base class validation.The base classes on the ol element are already validated in the non-solid test case (lines 61-64). To reduce duplication, you could:
- Remove this test and rely on the coverage in the non-solid test, or
- Expand this test to verify base classes persist under different prop combinations (e.g., with and without solid, or with additional props if supported).
src/components/FwbSpinner/tests/Spinner.spec.ts (3)
16-43: Replace if-else chain with a lookup object for better maintainability.The nested if-else statements make the test harder to maintain. Use a lookup object to map colors to their expected classes.
Apply this diff to refactor the color test:
it('renders with different colors', () => { - const colors = ['blue', 'gray', 'green', 'red', 'yellow', 'pink', 'purple'] as const + const colorClassMap = { + blue: 'fill-blue-600', + gray: 'fill-gray-600', + green: 'fill-green-500', + red: 'fill-red-600', + yellow: 'fill-yellow-400', + pink: 'fill-pink-600', + purple: 'fill-purple-600', + } as const - colors.forEach((color) => { + Object.entries(colorClassMap).forEach(([color, expectedClass]) => { const wrapper = mount(FwbSpinner, { props: { color }, }) const svg = wrapper.find('svg') const classes = svg.classes() - - if (color === 'blue') { - expect(classes).toContain('fill-blue-600') - } else if (color === 'gray') { - expect(classes).toContain('fill-gray-600') - } else if (color === 'green') { - expect(classes).toContain('fill-green-500') - } else if (color === 'red') { - expect(classes).toContain('fill-red-600') - } else if (color === 'yellow') { - expect(classes).toContain('fill-yellow-400') - } else if (color === 'pink') { - expect(classes).toContain('fill-pink-600') - } else if (color === 'purple') { - expect(classes).toContain('fill-purple-600') - } + expect(classes).toContain(expectedClass) }) })
45-73: Replace if-else chain with a lookup object for better maintainability.Similar to the color test, the nested if-else statements make this test harder to maintain. Use a lookup object to map sizes to their expected classes.
Apply this diff to refactor the size test:
it('renders with different sizes', () => { - const sizes = ['4', '6', '8', '10', '12'] as const + const sizeClassMap = { + '4': ['w-4', 'h-4'], + '6': ['w-6', 'h-6'], + '8': ['w-8', 'h-8'], + '10': ['w-10', 'h-10'], + '12': ['w-12', 'h-12'], + } as const - sizes.forEach((size) => { + Object.entries(sizeClassMap).forEach(([size, expectedClasses]) => { const wrapper = mount(FwbSpinner, { props: { size }, }) const svg = wrapper.find('svg') const classes = svg.classes() - - if (size === '4') { - expect(classes).toContain('w-4') - expect(classes).toContain('h-4') - } else if (size === '6') { - expect(classes).toContain('w-6') - expect(classes).toContain('h-6') - } else if (size === '8') { - expect(classes).toContain('w-8') - expect(classes).toContain('h-8') - } else if (size === '10') { - expect(classes).toContain('w-10') - expect(classes).toContain('h-10') - } else if (size === '12') { - expect(classes).toContain('w-12') - expect(classes).toContain('h-12') - } + expectedClasses.forEach(expectedClass => { + expect(classes).toContain(expectedClass) + }) }) })
99-108: Remove unnecessaryas anycast in custom color test
TheSpinnerColortype already accepts hex strings viaCustomColor, so you can passcolor: '#ff0000'withoutas any(or cast toSpinnerColorif needed).src/components/FwbToggle/tests/Toggle.spec.ts (4)
42-49: Consider a more semantic assertion for label rendering.The assertion
expect(wrapper.findAll('span')).toHaveLength(2)is coupled to the component's internal DOM structure. If the component adds or removes spans for styling purposes, this test will break even though the label functionality remains correct.Consider refactoring to test the label more semantically:
- expect(wrapper.text()).toContain('Enable notifications') - expect(wrapper.findAll('span')).toHaveLength(2) // toggle span + label span + expect(wrapper.text()).toContain('Enable notifications') + const label = wrapper.find('label') + expect(label.text()).toContain('Enable notifications')This approach verifies that the label text exists within the label element, which is more resilient to internal DOM changes.
60-82: Consider simplifying size assertions with a lookup map.The current if-else chain for size assertions is clear but somewhat verbose. You could simplify it with a lookup map for better maintainability.
it('renders with different sizes', () => { - const sizes = ['sm', 'md', 'lg'] as const + const sizeClasses = { + sm: ['w-9', 'h-5'], + md: ['w-11', 'h-6'], + lg: ['w-14', 'h-7'], + } as const - sizes.forEach((size) => { + Object.entries(sizeClasses).forEach(([size, expectedClasses]) => { const wrapper = mount(FwbToggle, { props: { size }, }) const toggleSpan = wrapper.find('span') const classes = toggleSpan.classes() - if (size === 'sm') { - expect(classes).toContain('w-9') - expect(classes).toContain('h-5') - } else if (size === 'md') { - expect(classes).toContain('w-11') - expect(classes).toContain('h-6') - } else if (size === 'lg') { - expect(classes).toContain('w-14') - expect(classes).toContain('h-7') - } + expectedClasses.forEach(cls => expect(classes).toContain(cls)) }) })
84-107: Consider simplifying color assertions with a lookup map.Similar to the size test, the color assertions could be simplified with a lookup map for better maintainability.
it('renders with different colors', () => { - const colors = ['blue', 'green', 'red', 'yellow', 'purple'] as const + const colorClasses = { + blue: 'peer-checked:bg-blue-600', + green: 'peer-checked:bg-green-600', + red: 'peer-checked:bg-red-600', + yellow: 'peer-checked:bg-yellow-400', + purple: 'peer-checked:bg-purple-600', + } as const - colors.forEach((color) => { + Object.entries(colorClasses).forEach(([color, expectedClass]) => { const wrapper = mount(FwbToggle, { props: { color, modelValue: true }, }) const toggleSpan = wrapper.find('span') const classes = toggleSpan.classes() - if (color === 'blue') { - expect(classes).toContain('peer-checked:bg-blue-600') - } else if (color === 'green') { - expect(classes).toContain('peer-checked:bg-green-600') - } else if (color === 'red') { - expect(classes).toContain('peer-checked:bg-red-600') - } else if (color === 'yellow') { - expect(classes).toContain('peer-checked:bg-yellow-400') - } else if (color === 'purple') { - expect(classes).toContain('peer-checked:bg-purple-600') - } + expect(classes).toContain(expectedClass) }) })
135-147: Consider consolidating duplicate test logic.This test overlaps significantly with the "handles reverse prop correctly" test (lines 109-119). Both verify the
flex-row-reverseclass whenreverseis true.Consider either:
- Removing this test if the previous one provides sufficient coverage, or
- Enhancing this test to cover additional aspects of label positioning not covered elsewhere.
The current duplication doesn't harm functionality but increases maintenance overhead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
.claude/settings.local.json(1 hunks).github/dependabot.yml(1 hunks).github/workflows/ci.yml(1 hunks).github/workflows/dependency-updates.yml(1 hunks)package.json(3 hunks)src/components/FwbBreadcrumb/tests/Breadcrumb.spec.ts(1 hunks)src/components/FwbFooter/FwbFooterBrand.vue(1 hunks)src/components/FwbFooter/FwbFooterLink.vue(1 hunks)src/components/FwbProgress/tests/Progress.spec.ts(1 hunks)src/components/FwbSidebar/FwbSidebarItemGroup.vue(1 hunks)src/components/FwbSpinner/tests/Spinner.spec.ts(1 hunks)src/components/FwbToggle/tests/Toggle.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .claude/settings.local.json
🔇 Additional comments (30)
src/components/FwbBreadcrumb/tests/Breadcrumb.spec.ts (5)
1-4: LGTM!The imports are correctly structured and include all necessary dependencies for component testing with Vue Test Utils and Vitest.
7-20: LGTM!This test case properly validates default rendering behavior, including structure, accessibility attributes, and slot content propagation.
22-45: LGTM!Comprehensive validation of solid styling, including dark mode classes and proper structure. The inline comments effectively document the test expectations.
47-65: LGTM!Effective use of negative assertions to ensure solid styling is not applied, along with validation of default spacing classes. This test properly complements the solid style test.
67-83: LGTM!Thorough validation of slot content propagation with multiple items, verifying both element count and text content.
src/components/FwbSidebar/FwbSidebarItemGroup.vue (1)
11-11: LGTM!The change to make the
borderprop explicitly optional is backward-compatible and aligns with the PR's TypeScript improvements objective. The default value offalseensures existing behavior is preserved.src/components/FwbFooter/FwbFooterLink.vue (1)
21-21: Verify the empty string default for optional href.The change to make
hrefoptional is backward-compatible. However, consider whether the empty string default (line 32) is appropriate:
- For
router-link: bindingto=""might cause routing issues or warnings.- For anchor tags:
href=""creates a link to the current page, which might not be the intended behavior.If the component is typically used with an explicit href, this is fine. Otherwise, consider using
undefinedas the default and conditionally rendering the attribute.src/components/FwbFooter/FwbFooterBrand.vue (1)
25-28: Verify the empty string defaults for optional props.The changes to make
href,src, andnameoptional are backward-compatible. However, consider the implications of the empty string defaults (lines 40-43):
href="": Creates an anchor linking to the current page, which might not be intended.src="": Creates an image with an empty source, resulting in a broken image icon (user-visible issue).name="": Renders an empty span, which is harmless.The
src=""default is particularly problematic as it creates a visible broken image. Consider:
- Using
undefinedas the default and conditionally rendering the image.- Or ensuring that consumers always provide the
srcprop when using this component.src/components/FwbProgress/tests/Progress.spec.ts (11)
1-6: LGTM!The imports and test suite setup are correct and follow Vitest best practices.
7-16: LGTM!The test correctly verifies default rendering behavior, including the 0% initial width.
18-25: LGTM!The test correctly verifies that the progress prop updates the width style attribute.
27-36: LGTM!The test correctly verifies label rendering.
38-53: LGTM!The test correctly verifies outside label positioning and progress percentage display, including structural validation with span count.
55-66: LGTM with a note on selector fragility.The test correctly verifies inside label positioning. The selector
'div > div > div'is fragile and depends on exact DOM nesting, but this is acceptable for component testing when unique identifiers aren't available.
68-94: LGTM!The test comprehensively covers all color variants with correct Tailwind class mappings. The use of
findAll()[1]assumes specific element ordering but is acceptable for component testing.
96-117: LGTM!The test comprehensively covers all size variants with correct height class mappings.
119-137: LGTM!The test correctly verifies that essential base classes are applied to both outer and inner elements, ensuring consistent styling.
139-151: LGTM!The test correctly handles edge cases for minimum (0%) and maximum (100%) progress values.
153-171: LGTM!The test correctly verifies that custom classes can be passed through and applied to the appropriate elements.
src/components/FwbSpinner/tests/Spinner.spec.ts (3)
7-14: LGTM!The default props test correctly verifies the SVG element's presence and essential attributes.
75-81: LGTM!The animation test is concise and correctly verifies the presence of the
animate-spinclass.
83-97: LGTM!The SVG structure test thoroughly verifies the element's attributes and path structure, including good use of optional chaining for safe array access.
src/components/FwbToggle/tests/Toggle.spec.ts (8)
1-5: LGTM!Imports are correctly structured and follow standard testing patterns for Vue components with Vitest.
7-19: LGTM!Comprehensive default rendering test with appropriate assertions for DOM structure, input type, and accessibility class.
21-31: LGTM!Well-structured test for v-model emit behavior with appropriate use of optional chaining to safely access emitted events.
33-40: LGTM!Clear test validating prop-to-DOM synchronization for v-model integration.
51-58: LGTM!Clear and correct test for disabled state handling.
109-119: LGTM!Clear test validating the reverse prop's effect on label positioning.
121-133: LGTM!Comprehensive test ensuring all essential base styling classes are applied correctly.
149-169: LGTM!Excellent end-to-end test for v-model functionality. The test properly simulates two-way binding by using the
onUpdate:modelValuecallback to update props, which mirrors real-world parent component behavior.
| reviewers: | ||
| - "audit-brands/maintainers" | ||
| assignees: | ||
| - "audit-brands/maintainers" | ||
| commit-message: | ||
| prefix: "chore(deps):" | ||
| include: "scope" | ||
| labels: | ||
| - "dependencies" | ||
| - "javascript" | ||
| # Group minor and patch updates | ||
| groups: | ||
| development-dependencies: | ||
| patterns: | ||
| - "@types/*" | ||
| - "@vue/*" | ||
| - "@vitejs/*" | ||
| - "eslint*" | ||
| - "vitest*" | ||
| - "typescript" | ||
| - "vite" | ||
| update-types: | ||
| - "minor" | ||
| - "patch" | ||
| production-dependencies: | ||
| patterns: | ||
| - "@vueuse/*" | ||
| - "vue" | ||
| - "tailwindcss" | ||
| - "@tailwindcss/*" |
There was a problem hiding this comment.
Fix Dependabot reviewer/assignee settings.
Dependabot rejects configs where assignees contain team slugs (org/team) and it can only request reviews from teams that belong to the repo owner. As written, the workflow will fail to parse and no update PRs will be created. Please replace these entries with valid GitHub usernames or with a team slug that exists under themesberg, or simply drop them.
- reviewers:
- - "audit-brands/maintainers"
- assignees:
- - "audit-brands/maintainers"
+ # reviewers: []
+ # assignees: []📝 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.
| reviewers: | |
| - "audit-brands/maintainers" | |
| assignees: | |
| - "audit-brands/maintainers" | |
| commit-message: | |
| prefix: "chore(deps):" | |
| include: "scope" | |
| labels: | |
| - "dependencies" | |
| - "javascript" | |
| # Group minor and patch updates | |
| groups: | |
| development-dependencies: | |
| patterns: | |
| - "@types/*" | |
| - "@vue/*" | |
| - "@vitejs/*" | |
| - "eslint*" | |
| - "vitest*" | |
| - "typescript" | |
| - "vite" | |
| update-types: | |
| - "minor" | |
| - "patch" | |
| production-dependencies: | |
| patterns: | |
| - "@vueuse/*" | |
| - "vue" | |
| - "tailwindcss" | |
| - "@tailwindcss/*" | |
| # reviewers: [] | |
| # assignees: [] | |
| commit-message: | |
| prefix: "chore(deps):" | |
| include: "scope" | |
| labels: | |
| - "dependencies" | |
| - "javascript" | |
| # Group minor and patch updates | |
| groups: | |
| development-dependencies: | |
| patterns: | |
| - "@types/*" | |
| - "@vue/*" | |
| - "@vitejs/*" | |
| - "eslint*" | |
| - "vitest*" | |
| - "typescript" | |
| - "vite" | |
| update-types: | |
| - "minor" | |
| - "patch" | |
| production-dependencies: | |
| patterns: | |
| - "@vueuse/*" | |
| - "vue" | |
| - "tailwindcss" | |
| - "@tailwindcss/*" |
🤖 Prompt for AI Agents
.github/dependabot.yml around lines 11 to 40: the reviewers and assignees
entries use an external team slug ("audit-brands/maintainers") which Dependabot
rejects and reviewers must belong to the repository owner; replace those team
slugs with valid GitHub usernames or with a team slug that exists under the repo
owner (e.g., themesberg/<team>) or remove the reviewers/assignees keys entirely;
ensure assignees are individual usernames (not teams) and reviewers reference
only teams that actually belong to the repo owner so the Dependabot config
parses and PRs can be created.
| jobs: | ||
| update-dependencies: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '20.x' | ||
| cache: 'npm' | ||
|
|
||
| - name: Check for updates | ||
| id: updates | ||
| run: | | ||
| npm outdated --json > outdated.json || true | ||
| if [ -s outdated.json ] && [ "$(cat outdated.json)" != "{}" ]; then | ||
| echo "has_updates=true" >> $GITHUB_OUTPUT | ||
| echo "Updates available:" | ||
| cat outdated.json | ||
| else | ||
| echo "has_updates=false" >> $GITHUB_OUTPUT | ||
| echo "No updates available" | ||
| fi | ||
|
|
||
| - name: Create dependency update issue | ||
| if: steps.updates.outputs.has_updates == 'true' | ||
| uses: actions/github-script@v7 | ||
| with: | ||
| script: | | ||
| const fs = require('fs'); | ||
| const outdated = JSON.parse(fs.readFileSync('outdated.json', 'utf8')); | ||
|
|
||
| let body = '# Dependency Updates Available\n\n'; | ||
| body += 'The following dependencies have updates available:\n\n'; | ||
| body += '| Package | Current | Wanted | Latest |\n'; | ||
| body += '|---------|---------|--------|--------|\n'; | ||
|
|
||
| for (const [name, info] of Object.entries(outdated)) { | ||
| body += `| ${name} | ${info.current} | ${info.wanted} | ${info.latest} |\n`; | ||
| } | ||
|
|
||
| body += '\n## Next Steps\n'; | ||
| body += '1. Review the changes for each package\n'; | ||
| body += '2. Test updates in a development environment\n'; | ||
| body += '3. Update dependencies with `npm update`\n'; | ||
| body += '4. Run tests to ensure compatibility\n'; | ||
| body += '\n*This issue was automatically created by the dependency update workflow.*'; | ||
|
|
||
| // Check if an issue already exists | ||
| const issues = await github.rest.issues.listForRepo({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| state: 'open', | ||
| labels: ['dependencies'] | ||
| }); | ||
|
|
||
| const existingIssue = issues.data.find(issue => | ||
| issue.title.includes('Dependency Updates Available') | ||
| ); | ||
|
|
||
| if (existingIssue) { | ||
| // Update existing issue | ||
| await github.rest.issues.update({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: existingIssue.number, | ||
| body: body | ||
| }); | ||
| console.log(`Updated existing issue #${existingIssue.number}`); | ||
| } else { | ||
| // Create new issue | ||
| const issue = await github.rest.issues.create({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| title: 'Dependency Updates Available', | ||
| body: body, | ||
| labels: ['dependencies', 'maintenance'] | ||
| }); | ||
| console.log(`Created new issue #${issue.data.number}`); | ||
| } No newline at end of file |
There was a problem hiding this comment.
Grant issues: write to the workflow.
actions/github-script cannot create or update issues with the default token permissions (only contents: read), so the job will always fail at the GitHub API call. Add explicit permissions before the jobs block:
on:
schedule:
- cron: '0 9 * * 1'
workflow_dispatch:
+permissions:
+ contents: read
+ issues: write
+
jobs:
update-dependencies:📝 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.
| jobs: | |
| update-dependencies: | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Checkout code | |
| uses: actions/checkout@v4 | |
| with: | |
| token: ${{ secrets.GITHUB_TOKEN }} | |
| - name: Setup Node.js | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: '20.x' | |
| cache: 'npm' | |
| - name: Check for updates | |
| id: updates | |
| run: | | |
| npm outdated --json > outdated.json || true | |
| if [ -s outdated.json ] && [ "$(cat outdated.json)" != "{}" ]; then | |
| echo "has_updates=true" >> $GITHUB_OUTPUT | |
| echo "Updates available:" | |
| cat outdated.json | |
| else | |
| echo "has_updates=false" >> $GITHUB_OUTPUT | |
| echo "No updates available" | |
| fi | |
| - name: Create dependency update issue | |
| if: steps.updates.outputs.has_updates == 'true' | |
| uses: actions/github-script@v7 | |
| with: | |
| script: | | |
| const fs = require('fs'); | |
| const outdated = JSON.parse(fs.readFileSync('outdated.json', 'utf8')); | |
| let body = '# Dependency Updates Available\n\n'; | |
| body += 'The following dependencies have updates available:\n\n'; | |
| body += '| Package | Current | Wanted | Latest |\n'; | |
| body += '|---------|---------|--------|--------|\n'; | |
| for (const [name, info] of Object.entries(outdated)) { | |
| body += `| ${name} | ${info.current} | ${info.wanted} | ${info.latest} |\n`; | |
| } | |
| body += '\n## Next Steps\n'; | |
| body += '1. Review the changes for each package\n'; | |
| body += '2. Test updates in a development environment\n'; | |
| body += '3. Update dependencies with `npm update`\n'; | |
| body += '4. Run tests to ensure compatibility\n'; | |
| body += '\n*This issue was automatically created by the dependency update workflow.*'; | |
| // Check if an issue already exists | |
| const issues = await github.rest.issues.listForRepo({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| state: 'open', | |
| labels: ['dependencies'] | |
| }); | |
| const existingIssue = issues.data.find(issue => | |
| issue.title.includes('Dependency Updates Available') | |
| ); | |
| if (existingIssue) { | |
| // Update existing issue | |
| await github.rest.issues.update({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: existingIssue.number, | |
| body: body | |
| }); | |
| console.log(`Updated existing issue #${existingIssue.number}`); | |
| } else { | |
| // Create new issue | |
| const issue = await github.rest.issues.create({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| title: 'Dependency Updates Available', | |
| body: body, | |
| labels: ['dependencies', 'maintenance'] | |
| }); | |
| console.log(`Created new issue #${issue.data.number}`); | |
| } | |
| on: | |
| schedule: | |
| - cron: '0 9 * * 1' | |
| workflow_dispatch: | |
| permissions: | |
| contents: read | |
| issues: write | |
| jobs: | |
| update-dependencies: | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Checkout code | |
| uses: actions/checkout@v4 | |
| with: | |
| token: ${{ secrets.GITHUB_TOKEN }} | |
| - name: Setup Node.js | |
| uses: actions/setup-node@v4 | |
| with: | |
| node-version: '20.x' | |
| cache: 'npm' | |
| - name: Check for updates | |
| id: updates | |
| run: | | |
| npm outdated --json > outdated.json || true | |
| if [ -s outdated.json ] && [ "$(cat outdated.json)" != "{}" ]; then | |
| echo "has_updates=true" >> $GITHUB_OUTPUT | |
| echo "Updates available:" | |
| cat outdated.json | |
| else | |
| echo "has_updates=false" >> $GITHUB_OUTPUT | |
| echo "No updates available" | |
| fi | |
| - name: Create dependency update issue | |
| if: steps.updates.outputs.has_updates == 'true' | |
| uses: actions/github-script@v7 | |
| with: | |
| script: | | |
| const fs = require('fs'); | |
| const outdated = JSON.parse(fs.readFileSync('outdated.json', 'utf8')); | |
| let body = '# Dependency Updates Available\n\n'; | |
| body += 'The following dependencies have updates available:\n\n'; | |
| body += '| Package | Current | Wanted | Latest |\n'; | |
| body += '|---------|---------|--------|--------|\n'; | |
| for (const [name, info] of Object.entries(outdated)) { | |
| body += `| ${name} | ${info.current} | ${info.wanted} | ${info.latest} |\n`; | |
| } | |
| body += '\n## Next Steps\n'; | |
| body += '1. Review the changes for each package\n'; | |
| body += '2. Test updates in a development environment\n'; | |
| body += '3. Update dependencies with `npm update`\n'; | |
| body += '4. Run tests to ensure compatibility\n'; | |
| body += '\n*This issue was automatically created by the dependency update workflow.*'; | |
| // Check if an issue already exists | |
| const issues = await github.rest.issues.listForRepo({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| state: 'open', | |
| labels: ['dependencies'] | |
| }); | |
| const existingIssue = issues.data.find(issue => | |
| issue.title.includes('Dependency Updates Available') | |
| ); | |
| if (existingIssue) { | |
| // Update existing issue | |
| await github.rest.issues.update({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: existingIssue.number, | |
| body: body | |
| }); | |
| console.log(`Updated existing issue #${existingIssue.number}`); | |
| } else { | |
| // Create new issue | |
| const issue = await github.rest.issues.create({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| title: 'Dependency Updates Available', | |
| body: body, | |
| labels: ['dependencies', 'maintenance'] | |
| }); | |
| console.log(`Created new issue #${issue.data.number}`); | |
| } |
🤖 Prompt for AI Agents
.github/workflows/dependency-updates.yml lines 9-92: the workflow uses
actions/github-script to create/update issues but lacks explicit permissions so
the default token won't allow issue writes; add a top-level permissions block
(placed before jobs:) that grants issues: write (e.g., permissions: issues:
write) so the workflow can create and update GitHub issues, keeping or
explicitly setting required other permissions like contents: read if needed.
- Add missing newlines at end of files - Fix inconsistent quote props in Toggle test - Ensures CI passes linting checks
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/components/FwbBreadcrumb/tests/Breadcrumb.spec.ts (3)
85-92: Consider removing redundant accessibility test.This test duplicates coverage from the first test case (lines 15-17), which already verifies the
aria-labelattribute. Consolidating reduces maintenance overhead.Consider removing this test or expanding it to cover additional accessibility concerns (e.g., role attributes, keyboard navigation) if applicable.
94-106: Consider removing redundant base classes test.This test duplicates class assertions from the third test case (lines 61-64). The same base classes (
inline-flex,items-center,space-x-1,md:space-x-3) are already verified in the "renders with default (non-solid) style" test.Consider consolidating to avoid duplicate maintenance.
6-107: Suggest adding edge case coverage.While the current tests cover the main functionality well, consider adding tests for edge cases to make the suite more robust:
Recommended additions:
- Test behavior when slot content is empty or missing
- Test dark mode class application (dark:bg-gray-800, dark:border-gray-700 are checked in line 38-39 but not explicitly verified in dark mode context)
- Test with various combinations of props if other props exist beyond
solid- Test component behavior with deeply nested breadcrumb items
These additions would provide more comprehensive coverage and catch potential edge case bugs.
src/components/FwbToggle/tests/Toggle.spec.ts (1)
60-107: Consider refactoring size and color tests for better maintainability.The if-else chains for validating size and color classes are repetitive. A data-driven approach using lookup objects would improve maintainability and reduce duplication.
Apply this refactor for the size test (lines 60-82):
it('renders with different sizes', () => { - const sizes = ['sm', 'md', 'lg'] as const + const sizeClasses = { + sm: ['w-9', 'h-5'], + md: ['w-11', 'h-6'], + lg: ['w-14', 'h-7'] + } as const - sizes.forEach((size) => { + Object.entries(sizeClasses).forEach(([size, classes]) => { const wrapper = mount(FwbToggle, { - props: { size }, + props: { size: size as 'sm' | 'md' | 'lg' }, }) const toggleSpan = wrapper.find('span') - const classes = toggleSpan.classes() - - if (size === 'sm') { - expect(classes).toContain('w-9') - expect(classes).toContain('h-5') - } else if (size === 'md') { - expect(classes).toContain('w-11') - expect(classes).toContain('h-6') - } else if (size === 'lg') { - expect(classes).toContain('w-14') - expect(classes).toContain('h-7') - } + const spanClasses = toggleSpan.classes() + classes.forEach(cls => { + expect(spanClasses).toContain(cls) + }) }) })Apply a similar refactor for the color test (lines 84-107):
it('renders with different colors', () => { - const colors = ['blue', 'green', 'red', 'yellow', 'purple'] as const + const colorClasses = { + blue: 'peer-checked:bg-blue-600', + green: 'peer-checked:bg-green-600', + red: 'peer-checked:bg-red-600', + yellow: 'peer-checked:bg-yellow-400', + purple: 'peer-checked:bg-purple-600' + } as const - colors.forEach((color) => { + Object.entries(colorClasses).forEach(([color, expectedClass]) => { const wrapper = mount(FwbToggle, { - props: { color, modelValue: true }, + props: { color: color as keyof typeof colorClasses, modelValue: true }, }) const toggleSpan = wrapper.find('span') const classes = toggleSpan.classes() - - if (color === 'blue') { - expect(classes).toContain('peer-checked:bg-blue-600') - } else if (color === 'green') { - expect(classes).toContain('peer-checked:bg-green-600') - } else if (color === 'red') { - expect(classes).toContain('peer-checked:bg-red-600') - } else if (color === 'yellow') { - expect(classes).toContain('peer-checked:bg-yellow-400') - } else if (color === 'purple') { - expect(classes).toContain('peer-checked:bg-purple-600') - } + expect(classes).toContain(expectedClass) }) })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/FwbBreadcrumb/tests/Breadcrumb.spec.ts(1 hunks)src/components/FwbProgress/tests/Progress.spec.ts(1 hunks)src/components/FwbSpinner/tests/Spinner.spec.ts(1 hunks)src/components/FwbToggle/tests/Toggle.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/FwbSpinner/tests/Spinner.spec.ts
- src/components/FwbProgress/tests/Progress.spec.ts
🔇 Additional comments (4)
src/components/FwbBreadcrumb/tests/Breadcrumb.spec.ts (2)
1-4: LGTM! Imports follow best practices.The test setup correctly imports from @vue/test-utils and vitest, aligned with the versions documented in the retrieved learnings.
6-83: Excellent core test coverage!The first four test cases provide solid coverage of the component's main functionality: default rendering, style variations (solid/non-solid), and slot content propagation. The tests are well-structured and follow testing best practices.
src/components/FwbToggle/tests/Toggle.spec.ts (2)
1-170: LGTM! Comprehensive test coverage for FwbToggle component.The test suite thoroughly covers all essential aspects of the Toggle component: default rendering, v-model behavior, prop handling, disabled state, size/color variations, reverse positioning, and base styling. The tests are well-organized, independent, and follow best practices for Vue component testing with vitest and @vue/test-utils.
149-169: Well-implemented v-model test.This test properly exercises the complete v-model cycle by manually implementing the update loop with
onUpdate:modelValue, which correctly simulates how v-model works in Vue. The test validates initial state, event emission, and prop synchronization—excellent coverage of the two-way binding behavior.
Summary
Phase 1: Security & Type Safety
anytypes with proper interfaces for better type safetyPhase 2: Testing & CI/CD Infrastructure
Impact
anytypes in source codeTest plan
Summary by CodeRabbit
New Features
Documentation
Tests
Chores