feat(icons): allow 3rd party packages to provide icon names for auto completion#3778
feat(icons): allow 3rd party packages to provide icon names for auto completion#3778
Conversation
📝 WalkthroughWalkthroughAdded an extendable Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
jgroth
force-pushed
the
autocomplete-icons
branch
from
b4a0507 to
0f3ea9d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/global/shared-types/icon.types.ts`:
- Around line 3-21: Add a Biome ignore comment immediately above the
intentionally empty IconNameRegistry interface to suppress the noEmptyInterface
rule; place a comment like /* biome-ignore no-empty-interface */ directly before
the export interface IconNameRegistry {} declaration so the linter ignores this
specific rule for that empty interface used for module augmentation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
src/components/action-bar/action-bar.types.tssrc/components/banner/banner.tsxsrc/components/button/button.tsxsrc/components/chip-set/chip-set.tsxsrc/components/dock/dock.types.tssrc/components/icon-button/icon-button.tsxsrc/components/input-field/input-field.tsxsrc/components/picker/picker.tsxsrc/components/profile-picture/profile-picture.tsxsrc/components/shortcut/shortcut.tsxsrc/components/split-button/split-button.tsxsrc/global/shared-types/icon.types.ts
🧰 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/input-field/input-field.tsxsrc/components/chip-set/chip-set.tsxsrc/components/banner/banner.tsxsrc/components/profile-picture/profile-picture.tsxsrc/components/picker/picker.tsxsrc/components/shortcut/shortcut.tsxsrc/components/split-button/split-button.tsxsrc/components/icon-button/icon-button.tsxsrc/components/button/button.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/input-field/input-field.tsxsrc/components/chip-set/chip-set.tsxsrc/components/banner/banner.tsxsrc/components/profile-picture/profile-picture.tsxsrc/components/picker/picker.tsxsrc/components/shortcut/shortcut.tsxsrc/components/split-button/split-button.tsxsrc/components/icon-button/icon-button.tsxsrc/components/button/button.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/input-field/input-field.tsxsrc/components/chip-set/chip-set.tsxsrc/components/banner/banner.tsxsrc/components/dock/dock.types.tssrc/components/profile-picture/profile-picture.tsxsrc/components/picker/picker.tsxsrc/components/shortcut/shortcut.tsxsrc/global/shared-types/icon.types.tssrc/components/split-button/split-button.tsxsrc/components/icon-button/icon-button.tsxsrc/components/action-bar/action-bar.types.tssrc/components/button/button.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/input-field/input-field.tsxsrc/components/chip-set/chip-set.tsxsrc/components/banner/banner.tsxsrc/components/profile-picture/profile-picture.tsxsrc/components/picker/picker.tsxsrc/components/shortcut/shortcut.tsxsrc/components/split-button/split-button.tsxsrc/components/icon-button/icon-button.tsxsrc/components/button/button.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/input-field/input-field.tsxsrc/components/chip-set/chip-set.tsxsrc/components/banner/banner.tsxsrc/components/profile-picture/profile-picture.tsxsrc/components/picker/picker.tsxsrc/components/shortcut/shortcut.tsxsrc/components/split-button/split-button.tsxsrc/components/icon-button/icon-button.tsxsrc/components/button/button.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: Kiarokh
Repo: Lundalogik/lime-elements PR: 3658
File: src/components/profile-picture/profile-picture.scss:120-124
Timestamp: 2025-09-03T07:17:23.267Z
Learning: Issue `#3666` in the lime-elements repository tracks the removal of a CSS hack in profile-picture component that overrides icon color via --limel-theme-on-surface-color, to be replaced with proper icon color support in limel-icon-button.
📚 Learning: 2025-04-25T13:32:56.396Z
Learnt from: adrianschmidt
Repo: Lundalogik/lime-elements PR: 3529
File: src/components/lime-ai-avatar/lime-ai-avatar.tsx:30-38
Timestamp: 2025-04-25T13:32:56.396Z
Learning: For StencilJS components in lime-elements, prefer using the `<Host>` element to wrap child elements in the render method instead of returning an array of elements, which eliminates the need for key attributes and follows StencilJS best practices.
Applied to files:
src/components/banner/banner.tsx
📚 Learning: 2025-04-25T13:32:56.396Z
Learnt from: adrianschmidt
Repo: Lundalogik/lime-elements PR: 3529
File: src/components/lime-ai-avatar/lime-ai-avatar.tsx:30-38
Timestamp: 2025-04-25T13:32:56.396Z
Learning: For StencilJS components in lime-elements, prefer using the `<Host>` element to wrap child elements in the render method instead of returning an array of elements. This follows the standard pattern in the codebase and eliminates the need for key attributes.
Applied to files:
src/components/banner/banner.tsx
📚 Learning: 2025-04-17T09:39:36.254Z
Learnt from: adrianschmidt
Repo: Lundalogik/lime-elements PR: 3530
File: src/components/text-editor/examples/text-editor-composite.tsx:4-7
Timestamp: 2025-04-17T09:39:36.254Z
Learning: For lime-elements, example files should import types from the public API using 'limetech/lime-elements' rather than defining duplicate types locally. This includes types like `EditorUiType` which are part of the component's public API.
Applied to files:
src/components/profile-picture/profile-picture.tsxsrc/components/picker/picker.tsxsrc/components/shortcut/shortcut.tsxsrc/global/shared-types/icon.types.ts
🧬 Code graph analysis (11)
src/components/input-field/input-field.tsx (1)
src/global/shared-types/icon.types.ts (1)
IconName(31-31)
src/components/chip-set/chip-set.tsx (1)
src/global/shared-types/icon.types.ts (1)
IconName(31-31)
src/components/banner/banner.tsx (1)
src/global/shared-types/icon.types.ts (1)
IconName(31-31)
src/components/dock/dock.types.ts (1)
src/global/shared-types/icon.types.ts (1)
IconName(31-31)
src/components/profile-picture/profile-picture.tsx (1)
src/global/shared-types/icon.types.ts (2)
IconName(31-31)Icon(38-68)
src/components/picker/picker.tsx (1)
src/global/shared-types/icon.types.ts (1)
IconName(31-31)
src/components/shortcut/shortcut.tsx (1)
src/global/shared-types/icon.types.ts (1)
IconName(31-31)
src/components/split-button/split-button.tsx (1)
src/global/shared-types/icon.types.ts (1)
IconName(31-31)
src/components/icon-button/icon-button.tsx (1)
src/global/shared-types/icon.types.ts (2)
IconName(31-31)Icon(38-68)
src/components/action-bar/action-bar.types.ts (1)
src/global/shared-types/icon.types.ts (2)
IconName(31-31)Icon(38-68)
src/components/button/button.tsx (1)
src/global/shared-types/icon.types.ts (2)
IconName(31-31)Icon(38-68)
🪛 Biome (2.1.2)
src/global/shared-types/icon.types.ts
[error] 21-21: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
⏰ 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). (2)
- GitHub Check: Docs / Publish Docs
- GitHub Check: Test
🔇 Additional comments (12)
src/components/action-bar/action-bar.types.ts (1)
1-19: LGTM — IconName integration is consistent.Line 19 aligns the icon typing with the new IconName registry without changing runtime behavior.
src/global/shared-types/icon.types.ts (1)
38-42: LGTM — Icon.name now uses IconName.Line 42 correctly aligns Icon with the new IconName type.
src/components/dock/dock.types.ts (1)
1-20: LGTM — DockItem.icon typed as IconName.Line 20 aligns with the new IconName registry and preserves behavior.
src/components/icon-button/icon-button.tsx (1)
7-28: LGTM — Icon prop now supports IconName | Icon.Line 28 correctly tightens typing while retaining compatibility.
src/components/banner/banner.tsx (1)
2-24: LGTM — Banner icon now typed as IconName.Line 24 aligns with the shared IconName type without changing behavior.
src/components/shortcut/shortcut.tsx (1)
5-32: Icon prop typing alignment looks good.Switching to
IconNamekeeps the runtime behavior intact while aligning the public API with the shared icon typing.src/components/chip-set/chip-set.tsx (1)
21-21: Leading icon typing update is consistent.Using
IconNameforleadingIconmatches the shared icon types and should improve autocomplete without behavioral changes.Also applies to: 174-174
src/components/button/button.tsx (1)
6-50: Button icon typing update looks correct.
IconName | Iconkeeps existing behavior and aligns the public API with the shared icon types.src/components/split-button/split-button.tsx (1)
4-45: SplitButton icon typing change is consistent.
IconNameusage matches the shared icon typing strategy without changing rendering behavior.src/components/profile-picture/profile-picture.tsx (1)
13-63: ProfilePicture icon typing update looks good.Switching to
IconName | Iconkeeps behavior intact while aligning with the shared icon typing.src/components/picker/picker.tsx (1)
26-26: IconName alignment looks good.This keeps the prop consistent with the shared icon typing for autocomplete and broader icon registry usage.
Also applies to: 85-85
src/components/input-field/input-field.tsx (1)
28-28: IconName prop typing is consistent and appropriate.Aligns the input field with the centralized icon type for safer autocomplete.
Also applies to: 135-135, 141-141
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
jgroth
force-pushed
the
autocomplete-icons
branch
from
0f3ea9d to
b795194
Compare
jgroth
force-pushed
the
autocomplete-icons
branch
2 times, most recently
from
74751e3 to
b6f4a28
Compare
jgroth
force-pushed
the
autocomplete-icons
branch
from
b6f4a28 to
d6b98cd
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/button/button.tsx (1)
152-152: 🧹 Nitpick | 🔵 TrivialUpdate
renderIconparameter type to match the new prop type.The private method signature still uses
string | Icon, but theiconprop is nowIconName | Icon. This should be aligned for consistency.Proposed fix
- private renderIcon(icon?: string | Icon) { + private renderIcon(icon?: IconName | Icon) {src/components/input-field/input-field.tsx (1)
761-761: 🧹 Nitpick | 🔵 TrivialConsider updating private helper signatures to use
IconNamefor consistency.
renderLinkIconandrenderTrailingIconaccepticon: string, but they now receive values originating fromIconName-typed properties. While this is type-safe (sinceIconNameis assignable tostring), updating these private signatures toIconNamewould keep the typing consistent throughout the file.Suggested diff
- private renderLinkIcon = (linkProps: LinkProperties, icon: string) => { + private renderLinkIcon = (linkProps: LinkProperties, icon: IconName) => {- private renderTrailingIcon = (icon: string) => { + private renderTrailingIcon = (icon: IconName) => {Also applies to: 779-779
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3778/ |
There was a problem hiding this comment.
Just making CodeRabbit's "outside the diff" comments more visible 🙂
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)src/components/button/button.tsx (1)>
152-152: 🧹 Nitpick | 🔵 TrivialUpdate
renderIconparameter type to match the new prop type.
The private method signature still usesstring | Icon, but theiconprop is nowIconName | Icon. This should be aligned for consistency.Proposed fix
- private renderIcon(icon?: string | Icon) { + private renderIcon(icon?: IconName | Icon) {src/components/input-field/input-field.tsx (1)>
761-761: 🧹 Nitpick | 🔵 TrivialConsider updating private helper signatures to use
IconNamefor consistency.
renderLinkIconandrenderTrailingIconaccepticon: string, but they now receive values originating fromIconName-typed properties. While this is type-safe (sinceIconNameis assignable tostring), updating these private signatures toIconNamewould keep the typing consistent throughout the file.Suggested diff
- private renderLinkIcon = (linkProps: LinkProperties, icon: string) => { + private renderLinkIcon = (linkProps: LinkProperties, icon: IconName) => {- private renderTrailingIcon = (icon: string) => { + private renderTrailingIcon = (icon: IconName) => {Also applies to: 779-779
There was a problem hiding this comment.
Addressed the outside-diff nitpicks for input-field.tsx (updated renderLinkIcon and renderTrailingIcon signatures to use IconName) in #3864. The button.tsx one was already fixed in the PR.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/dynamic-label/label.types.ts`:
- Line 1: The import in label.types.ts is inconsistent: replace the current
import of IconName and Icon from '../../interface' with the same module used
elsewhere ('../../global/shared-types/icon.types') so IconName and Icon are
imported from the direct shared types module; update the import statement that
references IconName and Icon to point to '../../global/shared-types/icon.types'
to match other files (stay mindful of the symbols IconName and Icon and the file
label.types.ts when editing).
In `@src/global/shared-types/file.types.ts`:
- Line 2: The import for Icon and IconName uses an overly verbose path; update
the import in file.types.ts to use the local sibling module path by replacing
the '../../global/shared-types/icon.types' import with './icon.types' so it
matches the other same-directory imports and preserves the Icon and IconName
symbols.
| @@ -1,4 +1,4 @@ | |||
| import { Icon } from '../../interface'; | |||
| import { IconName, Icon } from '../../interface'; | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent import path compared to other files in this PR.
All other files in this PR import IconName and Icon from '../../global/shared-types/icon.types', but this file imports from '../../interface'. While both are valid relative paths, consider aligning to the direct module path for consistency.
🤖 Prompt for AI Agents
In `@src/components/dynamic-label/label.types.ts` at line 1, The import in
label.types.ts is inconsistent: replace the current import of IconName and Icon
from '../../interface' with the same module used elsewhere
('../../global/shared-types/icon.types') so IconName and Icon are imported from
the direct shared types module; update the import statement that references
IconName and Icon to point to '../../global/shared-types/icon.types' to match
other files (stay mindful of the symbols IconName and Icon and the file
label.types.ts when editing).
There was a problem hiding this comment.
I left this one as-is — using ../../interface (the barrel export) is a valid pattern and the PR author may prefer it for this file. The other files import directly because they're closer to the source.
|
@jgroth, do you want me to ask @adrianschmidt-bot to finish this one for you? |
|
I went through the review comments and prepared fixes for all of them. However, since this branch lives on the upstream repo (not our fork), I don't have push access. Here's what I've prepared: Fixes addressing all 4 review comments:
Full diff (apply with
|
It seems it already tried to 😄 |
|
Yeah, I got a bit eager 😄 The diff above should be ready to apply if you want — just |
|
@jgroth Sorry, I didn't mean to bulldoze you. I guess I shouldn't have mentioned @adrianschmidt-bot in my question 😅 @adrianschmidt-bot You could open a PR targeting this PR's branch, then Johan can just pull it in, if he wants to. |
jgroth
force-pushed
the
autocomplete-icons
branch
from
c588dc0 to
006553d
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/header/header.tsx (1)
128-134:⚠️ Potential issue | 🟡 Minor
titleshould not be in thestyleprop.The
titleattribute is being set inside thestyleobject (line 133), which is incorrect. HTML attributes liketitlemust be separate props or attributes on the element, not CSS properties. Thelimel-iconcomponent does not accept atitleprop, so this will be ignored and the title attribute won't be set.Move
titleoutside the style object as a separate attribute on the element, or add title support to thelimel-iconcomponent if this functionality is needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/header/header.tsx` around lines 128 - 134, The title property is incorrectly placed inside the style object; remove the title entry from the style literal in header.tsx and add a separate title attribute on the limel-icon (or its wrapper) using (this.icon as Icon)?.title so the HTML title attribute is set (e.g., set title={(this.icon as Icon)?.title} on the element) while keeping the CSS custom properties (--limel-header-icon-color and --limel-header-icon-background-color) in the style object.
♻️ Duplicate comments (1)
src/global/shared-types/icon.types.ts (1)
21-21:⚠️ Potential issue | 🟠 MajorSuppress Biome
noEmptyInterfacefor the augmentation anchor.Line 21 is intentionally empty, but this pattern usually fails lint unless explicitly ignored.
Proposed fix
+// biome-ignore lint/suspicious/noEmptyInterface: used for module augmentation export interface IconNameRegistry {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/global/shared-types/icon.types.ts` at line 21, Add a linter suppression comment immediately above the empty interface declaration to allow the augmentation anchor; specifically, for the IconNameRegistry interface add a single-line disable for the Biome rule that flags empty interfaces (e.g., add a comment such as // eslint-disable-next-line biome/noEmptyInterface or // eslint-disable-next-line biome/no-empty-interface) so the empty export interface IconNameRegistry {} remains but the Biome noEmptyInterface rule is ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/header/header.tsx`:
- Around line 128-134: The title property is incorrectly placed inside the style
object; remove the title entry from the style literal in header.tsx and add a
separate title attribute on the limel-icon (or its wrapper) using (this.icon as
Icon)?.title so the HTML title attribute is set (e.g., set title={(this.icon as
Icon)?.title} on the element) while keeping the CSS custom properties
(--limel-header-icon-color and --limel-header-icon-background-color) in the
style object.
---
Duplicate comments:
In `@src/global/shared-types/icon.types.ts`:
- Line 21: Add a linter suppression comment immediately above the empty
interface declaration to allow the augmentation anchor; specifically, for the
IconNameRegistry interface add a single-line disable for the Biome rule that
flags empty interfaces (e.g., add a comment such as // eslint-disable-next-line
biome/noEmptyInterface or // eslint-disable-next-line biome/no-empty-interface)
so the empty export interface IconNameRegistry {} remains but the Biome
noEmptyInterface rule is ignored.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
etc/lime-elements.api.mdis excluded by!etc/lime-elements.api.md
📒 Files selected for processing (25)
src/components/action-bar/action-bar.types.tssrc/components/banner/banner.tsxsrc/components/button/button.tsxsrc/components/card/card.tsxsrc/components/chip-set/chip-set.tsxsrc/components/chip-set/chip.types.tssrc/components/chip/chip.tsxsrc/components/collapsible-section/collapsible-section.tsxsrc/components/dialog/dialog.types.tssrc/components/dock/dock.types.tssrc/components/dynamic-label/dynamic-label.tsxsrc/components/dynamic-label/label.types.tssrc/components/header/header.tsxsrc/components/icon-button/icon-button.tsxsrc/components/input-field/input-field.tsxsrc/components/list-item/list-item.types.tssrc/components/menu/menu.types.tssrc/components/picker/picker.tsxsrc/components/profile-picture/profile-picture.tsxsrc/components/select/option.types.tssrc/components/shortcut/shortcut.tsxsrc/components/split-button/split-button.tsxsrc/components/tab-bar/tab.types.tssrc/global/shared-types/file.types.tssrc/global/shared-types/icon.types.ts
jgroth
force-pushed
the
autocomplete-icons
branch
from
006553d to
fdcc416
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/global/shared-types/icon.types.ts (1)
21-21:⚠️ Potential issue | 🟠 MajorAdd suppression for intentional empty interface to avoid lint failure.
IconNameRegistryat Line 21 is intentionally empty, but this typically triggers Biome’snoEmptyInterfacerule unless explicitly ignored.Suggested fix
+// biome-ignore lint/suspicious/noEmptyInterface: intended module augmentation point export interface IconNameRegistry {}#!/bin/bash # Verify whether the empty interface is currently unsuppressed rg -n "export interface IconNameRegistry\\s*\\{\\s*\\}" src/global/shared-types/icon.types.ts -C2 rg -n "biome-ignore.*noEmptyInterface|lint/suspicious/noEmptyInterface" src/global/shared-types/icon.types.ts -C2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/global/shared-types/icon.types.ts` at line 21, The empty interface export IconNameRegistry is intentionally blank but triggers Biome's noEmptyInterface rule; suppress it by adding a Biome ignore comment immediately above the declaration (for example: /* biome-ignore noEmptyInterface */ or // biome-ignore noEmptyInterface) so the interface remains empty and the linter no longer errors; place the suppression directly above the export interface IconNameRegistry {} line to scope the rule exception to that declaration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/chip-set/chip-set.tsx`:
- Line 173: The leadingIcon property is typed as non-nullable IconName but
initialized to null and documented to allow null; update the type to include
null (e.g., change the declaration of leadingIcon in chip-set.tsx to use
IconName | null) so the TypeScript contract matches the documented/actual
behavior for the component.
---
Duplicate comments:
In `@src/global/shared-types/icon.types.ts`:
- Line 21: The empty interface export IconNameRegistry is intentionally blank
but triggers Biome's noEmptyInterface rule; suppress it by adding a Biome ignore
comment immediately above the declaration (for example: /* biome-ignore
noEmptyInterface */ or // biome-ignore noEmptyInterface) so the interface
remains empty and the linter no longer errors; place the suppression directly
above the export interface IconNameRegistry {} line to scope the rule exception
to that declaration.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
etc/lime-elements.api.mdis excluded by!etc/lime-elements.api.md
📒 Files selected for processing (25)
src/components/action-bar/action-bar.types.tssrc/components/banner/banner.tsxsrc/components/button/button.tsxsrc/components/card/card.tsxsrc/components/chip-set/chip-set.tsxsrc/components/chip-set/chip.types.tssrc/components/chip/chip.tsxsrc/components/collapsible-section/collapsible-section.tsxsrc/components/dialog/dialog.types.tssrc/components/dock/dock.types.tssrc/components/dynamic-label/dynamic-label.tsxsrc/components/dynamic-label/label.types.tssrc/components/header/header.tsxsrc/components/icon-button/icon-button.tsxsrc/components/input-field/input-field.tsxsrc/components/list-item/list-item.types.tssrc/components/menu/menu.types.tssrc/components/picker/picker.tsxsrc/components/profile-picture/profile-picture.tsxsrc/components/select/option.types.tssrc/components/shortcut/shortcut.tsxsrc/components/split-button/split-button.tsxsrc/components/tab-bar/tab.types.tssrc/global/shared-types/file.types.tssrc/global/shared-types/icon.types.ts
| */ | ||
| @Prop({ reflect: true }) | ||
| public leadingIcon: string = null; | ||
| public leadingIcon: IconName = null; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== strict/null settings in tsconfig files =="
fd 'tsconfig.*json' | while read -r file; do
echo "--- $file"
rg -n '"strict"\s*:|"strictNullChecks"\s*:' "$file" || true
done
echo
echo "== declaration under review =="
rg -n -C2 'public\s+leadingIcon\s*:\s*IconName\s*=\s*null' src/components/chip-set/chip-set.tsx || trueRepository: Lundalogik/lime-elements
Length of output: 384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tsconfig.json content (relevant parts) =="
cat tsconfig.json | head -30
echo
echo "== IconName type definition =="
rg -n 'type\s+IconName|interface\s+IconName|export.*IconName' -A 3
echo
echo "== Full context around leadingIcon (lines 165-180) =="
sed -n '165,180p' src/components/chip-set/chip-set.tsxRepository: Lundalogik/lime-elements
Length of output: 2843
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Complete tsconfig.json =="
cat tsconfig.json
echo
echo "== Import statement at line 22 =="
sed -n '1,30p' src/components/chip-set/chip-set.tsx | grep -n 'import.*IconName'Repository: Lundalogik/lime-elements
Length of output: 878
Include null in leadingIcon type to match the actual contract.
leadingIcon is initialized with null and documented as accepting null ("When the value is null, no leading icon is used"), but typed as non-nullable IconName. This creates a misleading type contract for consumers of this component.
Proposed fix
- public leadingIcon: IconName = null;
+ public leadingIcon: IconName | null = null;📝 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.
| public leadingIcon: IconName = null; | |
| public leadingIcon: IconName | null = null; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/chip-set/chip-set.tsx` at line 173, The leadingIcon property
is typed as non-nullable IconName but initialized to null and documented to
allow null; update the type to include null (e.g., change the declaration of
leadingIcon in chip-set.tsx to use IconName | null) so the TypeScript contract
matches the documented/actual behavior for the component.
jgroth
force-pushed
the
autocomplete-icons
branch
from
fdcc416 to
b5d7433
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/chip-set/chip-set.tsx (1)
169-173:⚠️ Potential issue | 🟠 Major
leadingIcontype contract is inconsistent with its actual null behavior (Line 173).The prop is documented and implemented as nullable, but typed as non-nullable
IconName. This creates an incorrect public contract for TS consumers.💡 Proposed fix
- public leadingIcon: IconName = null; + public leadingIcon: IconName | null = null;#!/bin/bash set -euo pipefail echo "== Type declaration and local usage for leadingIcon ==" rg -n -C2 'leadingIcon' src/components/chip-set/chip-set.tsx echo echo "== strict/null compiler settings ==" fd 'tsconfig*.json' -x sh -c 'echo "--- $1"; rg -n "\"strict\"|\"strictNullChecks\"" "$1" || true' sh {}Expected verification outcome:
- Confirms Line 173 non-nullable type +
nullinitializer and null-based runtime checks.- If
strictNullChecksis enabled in any active tsconfig, this is a direct typing error; otherwise it is still a misleading public API.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/chip-set/chip-set.tsx` around lines 169 - 173, The public prop leadingIcon is typed as non-nullable IconName but initialized and checked as null at runtime; update the type contract to reflect nullability by changing its type to IconName | null (in src/components/chip-set/chip-set.tsx, symbol leadingIcon) and adjust any JSDoc to state it can be null; run TypeScript with strictNullChecks to verify no downstream callers assume a non-null value and add null-safe checks where leadingIcon is used if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/chip-set/chip-set.tsx`:
- Around line 169-173: The public prop leadingIcon is typed as non-nullable
IconName but initialized and checked as null at runtime; update the type
contract to reflect nullability by changing its type to IconName | null (in
src/components/chip-set/chip-set.tsx, symbol leadingIcon) and adjust any JSDoc
to state it can be null; run TypeScript with strictNullChecks to verify no
downstream callers assume a non-null value and add null-safe checks where
leadingIcon is used if needed.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
etc/lime-elements.api.mdis excluded by!etc/lime-elements.api.md
📒 Files selected for processing (25)
src/components/action-bar/action-bar.types.tssrc/components/banner/banner.tsxsrc/components/button/button.tsxsrc/components/card/card.tsxsrc/components/chip-set/chip-set.tsxsrc/components/chip-set/chip.types.tssrc/components/chip/chip.tsxsrc/components/collapsible-section/collapsible-section.tsxsrc/components/dialog/dialog.types.tssrc/components/dock/dock.types.tssrc/components/dynamic-label/dynamic-label.tsxsrc/components/dynamic-label/label.types.tssrc/components/header/header.tsxsrc/components/icon-button/icon-button.tsxsrc/components/input-field/input-field.tsxsrc/components/list-item/list-item.types.tssrc/components/menu/menu.types.tssrc/components/picker/picker.tsxsrc/components/profile-picture/profile-picture.tsxsrc/components/select/option.types.tssrc/components/shortcut/shortcut.tsxsrc/components/split-button/split-button.tsxsrc/components/tab-bar/tab.types.tssrc/global/shared-types/file.types.tssrc/global/shared-types/icon.types.ts
3 participants
fix Lundalogik/crm-client#622
Summary by CodeRabbit
How to test
@limetech/lime-elements@limetech/lime-elementsinside@lundalogik/lime-icons8@lundalogik/lime-icons8@limetech/lime-elementsand@lundalogik/lime-icons8inside any package (or test in https://github.com/Lundalogik/lime-crm-building-blocks/pull/1264 or https://github.com/Lundalogik/lime-crm-components/pull/3994)import '@lundalogik/lime-icons8';somewhereReview:
Browsers tested:
(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)
Windows:
Linux:
macOS:
Mobile: