-
Notifications
You must be signed in to change notification settings - Fork 16
fix(radio-button-group): use internal import path for types #3695
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
Conversation
Update import path from '@limetech/lime-elements' to '../../components'. This change is necessary because lime-elements cannot import from itself. Using the package name '@limetech/lime-elements' in imports within the src/ directory would create circular dependencies and cause issues with the components.d.ts file generation. Co-Authored-By: Claude <[email protected]>
📝 WalkthroughWalkthroughUpdated the import of LimelListCustomEvent in src/components/radio-button-group/radio-button-group.tsx from an external package to a local internal path. No other code, API, or logic changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 |
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3695/ |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/components/radio-button-group/radio-button-group.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/wrap-multiple-jsx-elements-in-host-not-in-array.mdc)
When returning multiple JSX elements from the
rendermethod, never wrap them in an array literal. Instead, always wrap them in the special<Host>element. When there is already a single top-level element, it does not have to be wrapped in<Host>.
Files:
src/components/radio-button-group/radio-button-group.tsx
⚙️ CodeRabbit configuration file
**/*.tsx: Our.tsxfiles are typically using StencilJS, not React. When a developer wants to return multiple top-level JSX elements from therendermethod, they will sometimes wrap them in an array literal. In these cases, rather than recommending they addkeyproperties to the elements, recommend removing the hardcoded array literal. Recommend wrapping the elements in StencilJS's special<Host>element.
Files:
src/components/radio-button-group/radio-button-group.tsx
**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{ts,tsx}: Imports from other files in the same module (lime-elements) must use relative paths. Using absolute paths for imports will cause the production build to fail.
Files:
src/components/radio-button-group/radio-button-group.tsx
**/*.{tsx,scss}
⚙️ CodeRabbit configuration file
**/*.{tsx,scss}: Almost all our components use shadow-DOM. Therefore, we have no need of BEM-style class names in our CSS.
Files:
src/components/radio-button-group/radio-button-group.tsx
src/components/**/*.tsx
⚙️ CodeRabbit configuration file
src/components/**/*.tsx: When contributors add new props to existing components in the lime-elements repository, they should always add documentation examples that demonstrate the new prop's usage and explain how it works. This helps with user adoption, feature discoverability, and maintains comprehensive documentation.
Files:
src/components/radio-button-group/radio-button-group.tsx
🧠 Learnings (3)
📚 Learning: 2025-08-12T14:01:27.846Z
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3638
File: src/components/radio-button-group/radio-button-group.tsx:91-91
Timestamp: 2025-08-12T14:01:27.846Z
Learning: The `limel-radio-button-group` component allows deselection of radio buttons (unlike native radio buttons), which improves UX by letting users leave questions unanswered, but diverges from standard radio button behavior expectations.
Applied to files:
src/components/radio-button-group/radio-button-group.tsx
📚 Learning: 2025-08-26T08:42:17.994Z
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3628
File: src/components/list-item/list-item.tsx:316-321
Timestamp: 2025-08-26T08:42:17.994Z
Learning: In src/components/list-item/list-item.tsx, the RadioButtonTemplate and CheckboxTemplate components are used purely for visualization and receive their checked state from the parent component's selected prop. The actual interaction is handled by a centralized onClick handler on the Host element, making the entire list item clickable. Adding onChange handlers to these templates would be redundant and potentially problematic.
Applied to files:
src/components/radio-button-group/radio-button-group.tsx
📚 Learning: 2025-08-07T14:39:00.053Z
Learnt from: Kiarokh
PR: Lundalogik/lime-elements#3638
File: src/components/radio-button-group/radio-button-group.tsx:32-32
Timestamp: 2025-08-07T14:39:00.053Z
Learning: The `limel-radio-button-group` component uses `shadow: false` because it's a pure wrapper around `limel-list` with no component-specific styles, following the pattern of other wrapper components like `limel-action-bar-overflow-menu`.
Applied to files:
src/components/radio-button-group/radio-button-group.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Docs / Publish Docs
- GitHub Check: Test
- GitHub Check: Build
| import { Component, Event, EventEmitter, h, Prop } from '@stencil/core'; | ||
| import { ListItem, ListSeparator } from '../list-item/list-item.types'; | ||
| import { LimelListCustomEvent } from '@limetech/lime-elements'; | ||
| import { LimelListCustomEvent } from '../../components'; |
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.
🧩 Analysis chain
LGTM! Import path correctly updated to use relative path.
The change from @limetech/lime-elements to ../../components aligns with the coding guidelines, which require relative paths for same-module imports to avoid circular dependencies and build failures.
As per coding guidelines, verify that the import path resolves correctly:
🏁 Script executed:
#!/bin/bash
# Description: Verify that LimelListCustomEvent is exported from ../../components (src/components/index.ts or similar)
# Find the barrel export file
fd -t f "index.ts" src/components -d 1 --exec cat {} \; | rg -n "LimelListCustomEvent"
# Also check if there's a components.ts or components.d.ts
fd -t f -g "components.*" src -d 1 --exec sh -c 'echo "=== {} ===" && cat {}'Length of output: 0
🏁 Script executed:
#!/bin/bash
echo "=== Searching for LimelListCustomEvent occurrences in src/components ==="
rg -n "LimelListCustomEvent" src/components || echo "No occurrences found"
echo ""
echo "=== Listing index.ts files under src/components ==="
fd -t f index.ts src/components || echo "No index.ts files found"
echo ""
echo "=== Checking barrel exports in index.ts files ==="
fd -t f index.ts src/components -x sh -c 'echo "== File: {} =="; grep -n "export .*LimelListCustomEvent" {} || echo "No exports in {}"'Length of output: 2533
Fix import path for LimelListCustomEvent
../../components has no barrel export for this type. Import it directly from its definition (for example, ../list/list.types) or add LimelListCustomEvent to your src/components/index.ts barrel.
🤖 Prompt for AI Agents
In src/components/radio-button-group/radio-button-group.tsx around line 3, the
import for LimelListCustomEvent is pointing to a barrel that doesn’t export that
type; change the import to the type’s actual file (e.g., import {
LimelListCustomEvent } from '../list/list.types') or alternatively add
LimelListCustomEvent to src/components/index.ts so the barrel export is
available.
|
🎉 This PR is included in version 38.24.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Update import path from '@limetech/lime-elements' to '../../components'.
This change is necessary because lime-elements cannot import from itself. Using the package name '@limetech/lime-elements' in imports within the src/ directory would create circular dependencies and cause issues with the components.d.ts file generation.
Summary by CodeRabbit
Review:
Browsers tested:
(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)
Windows:
Linux:
macOS:
Mobile: