-
Notifications
You must be signed in to change notification settings - Fork 418
feat(ConfigProvider): support global configuration #2921
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
太强了
|
review 下设计是否合理? |
9549bdb
to
82150db
Compare
children?: ReactNode; | ||
} | ||
|
||
export default function ConfigProvider({ children, ...value }: ConfigProviderProps) { |
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.
加上 Memo 避免不必要的二次渲染?
const value = useMemo(() => props, [props]);
return (
<ConfigContext.Provider value={value}>
{children}
</ConfigContext.Provider>
);
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.
也可以合并配置之后在 makeChartComp 中实现。
import type { ViolinConfig } from './components/violin'; | ||
import type { WaterfallConfig } from './components/waterfall'; | ||
import type { WordCloudConfig } from './components/wordCloud'; | ||
import type { CommonConfig } from './interface'; |
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.
和 components/index.ts
有些重复,可以看下如何优化。
设计上没什么问题,目前的一个问题是配置变更时所有组件会重新渲染,加上 Memo 就好了。 |
@lxfu1 用 |
嗯嗯,那就先忽略,确实变化频率不高。 |
|
支持全局配置。