[WIP] ♻️ refactor: Use dynamic import#152
Draft
canisminor1990 wants to merge 3 commits intomasterfrom
Draft
Conversation
Contributor
Reviewer's GuideThis refactor replaces all static icon imports with dynamic imports across provider and model mappings, and updates the icon-rendering components (ProviderIcon, ModelIcon, ProviderCombine) to lazy-load icons using useEffect, Suspense, and a LoadingPlaceholder. It also introduces a new DynamicCombine component, adds Combine support in the Google and Anthropic packages (including style tweaks and docs), and flips the hasCombine flags in toc.ts. Sequence diagram for dynamic icon loading in ProviderIcon/ModelIcon/ProviderCombinesequenceDiagram
participant Component as ProviderIcon/ModelIcon/ProviderCombine
participant Mapping as providerMappings/modelMappings
participant IconModule as IconComponent
participant User as actor User
User->>Component: Render with provider/model
Component->>Mapping: Find mappingItem by keyword
Component->>Component: useEffect triggers
Component->>IconModule: mappingItem.iconImport() (dynamic import)
IconModule-->>Component: Promise resolves with IconComponent
Component->>Component: set IconComponent, set loading=false
Component->>User: Render Suspense/LoadingPlaceholder, then IconComponent
Sequence diagram for DynamicCombine icon loadingsequenceDiagram
participant DynamicCombine
participant LeftIconModule as LeftIcon
participant RightIconModule as RightIcon
participant User as actor User
User->>DynamicCombine: Render with leftImport, rightImport
DynamicCombine->>LeftIconModule: leftImport() (dynamic import)
DynamicCombine->>RightIconModule: rightImport() (dynamic import)
LeftIconModule-->>DynamicCombine: Promise resolves with LeftIcon
RightIconModule-->>DynamicCombine: Promise resolves with RightIcon
DynamicCombine->>DynamicCombine: set LeftIcon, RightIcon, loading=false
DynamicCombine->>User: Render Suspense/LoadingPlaceholder, then icons
Class diagram for dynamic icon loading refactorclassDiagram
class ProviderIcon {
+ProviderIconProps
+useState(IconComponent, loading)
+useMemo(mappingItem)
+useEffect(load iconImport)
+render: Suspense, LoadingPlaceholder, IconComponent
}
class ModelIcon {
+ModelIconProps
+useState(IconComponent, loading)
+useMemo(mappingItem)
+useEffect(load iconImport)
+render: Suspense, LoadingPlaceholder, IconComponent
}
class ProviderCombine {
+ProviderCombineProps
+useState(IconComponent, loading)
+useMemo(mappingItem)
+useEffect(load iconImport)
+render: Suspense, LoadingPlaceholder, IconComponent or mappingItem.Combine
}
class DynamicCombine {
+DynamicCombineProps
+useState(LeftIcon, RightIcon, loading)
+useEffect(load leftImport, rightImport)
+render: Suspense, LoadingPlaceholder, LeftIcon, RightIcon
}
ProviderIcon --> "1" ProviderMapping
ModelIcon --> "1" ModelMapping
ProviderCombine --> "1" ProviderMapping
ProviderCombine --> "1" DynamicCombine
DynamicCombine --> "1" LoadingPlaceholder
ProviderIcon --> "1" LoadingPlaceholder
ModelIcon --> "1" LoadingPlaceholder
ProviderCombine --> "1" LoadingPlaceholder
Class diagram for Google and Anthropic icon packages with Combine supportclassDiagram
class Google {
+Brand
+BrandColor
+Color
+Combine
+Avatar
+colorPrimary
+title
}
class Anthropic {
+Text
+Combine
+Avatar
+colorPrimary
+title
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Member
|
👍 @canisminor1990 |
canisminor1990
force-pushed
the
pref/dynamic-import
branch
from
5758302 to
d0fa6e1
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @canisminor1990 - I've reviewed your changes - here's some feedback:
- There’s a lot of repeated logic for loading placeholders and dynamic imports in ProviderIcon, ModelIcon, and ProviderCombine—consider extracting this into a shared hook or component to DRY things up.
- You should guard against state updates on unmounted components during async loads (e.g. using an isMounted flag or an AbortController) to avoid potential memory leaks or console errors.
- Instead of managing dynamic imports with useState/useEffect in each component, you could leverage React.lazy with Suspense for cleaner code and built-in loading handling.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s a lot of repeated logic for loading placeholders and dynamic imports in ProviderIcon, ModelIcon, and ProviderCombine—consider extracting this into a shared hook or component to DRY things up.
- You should guard against state updates on unmounted components during async loads (e.g. using an isMounted flag or an AbortController) to avoid potential memory leaks or console errors.
- Instead of managing dynamic imports with useState/useEffect in each component, you could leverage React.lazy with Suspense for cleaner code and built-in loading handling.
## Individual Comments
### Comment 1
<location> `src/features/ProviderIcon/index.tsx:35` </location>
<code_context>
({ model: originModel, size = 12, type = 'avatar', shape, ...rest }) => {
- const Render = useMemo(() => {
- if (!originModel) return;
+ const [IconComponent, setIconComponent] = useState<any>(null);
+ const [loading, setLoading] = useState(false);
+
+ const mappingItem = useMemo(() => {
</code_context>
<issue_to_address>
Potential race condition if iconImport changes rapidly.
To prevent outdated imports from overwriting newer ones, implement a cancellation mechanism or use a ref to ensure state updates only occur for the latest provider change.
</issue_to_address>
### Comment 2
<location> `src/Google/index.ts:21` </location>
<code_context>
Icons.Color = Color;
Icons.Brand = BrandMono;
Icons.BrandColor = BrandColor;
+Icons.Brand = BrandMono;
</code_context>
<issue_to_address>
Redundant assignment to Icons.Brand.
The second assignment to Icons.Brand can be removed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
src/features/ProviderIcon/index.tsx
Outdated
Comment on lines
+35
to
+36
| const [IconComponent, setIconComponent] = useState<any>(null); | ||
| const [loading, setLoading] = useState(false); |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Potential race condition if iconImport changes rapidly.
To prevent outdated imports from overwriting newer ones, implement a cancellation mechanism or use a ref to ensure state updates only occur for the latest provider change.
commit: |
canisminor1990
changed the title
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Note
对性能影响收益未知，暂时搁置
💻 变更类型 | Change Type
动态导入性能优化
🎯 核心改进
对
ModelIconProviderIconProviderCombine进行动态导入和智能缓存，提升 Icon 大量聚合后的性能。🚀 关键技术优化
1. 动态导入替代静态导入
2. O(1) 查找算法
3. 统一的 Hook 架构
🛠️ 架构升级
新增核心模块
useMappingCache.ts- O(1) 查找缓存useIconLoader.ts- 统一图标加载逻辑LoadingPlaceholder.tsx- 智能加载占位符代码重构亮点
🔧 使用方式
完全兼容的 API
预加载优化（可选）