-
Notifications
You must be signed in to change notification settings - Fork 115
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
feat: add common Token type #902
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 67febc0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #902 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 708 708
Lines 21461 21461
Branches 1214 1214
=========================================
Hits 21461 21461 ☔ View full report in Codecov by Sentry. |
|
packages/common/src/types.ts
Outdated
@@ -37,6 +37,7 @@ export type BalanceMetadata = { | |||
export interface Chain { | |||
id: ChainIds | number; | |||
name: string; | |||
type?: string; |
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.
这个搞一个枚举类型出来吧
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.
VMType: 'EVM'|'SOL'
这样吗
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.
这个搞一个枚举类型出来吧
加好了
packages/common/src/types.ts
Outdated
decimal: number; | ||
}; | ||
|
||
export type ChainToken = TokenBase & { |
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.
感觉不需要这么分,就统一用数组就好了,
type Token = {
name: string;
symbol: string;
icon: React.ReactNode,
decimal: number;
availableChains: [{
chain: Chain,
contract: string;
}],
};
这样是不是就好了?
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.
感觉不需要这么分,就统一用数组就好了,
type Token = { name: string; symbol: string; icon: React.ReactNode, decimal: number; availableChains: [{ chain: Chain, contract: string; }], };这样是不是就好了?
昨天和武田对了下这个场景
他那边是属于 Token 确定,链可变,也就是 多链单 Token
我这边是属于链确定,Token 可变,是单链多 Token
如果定成一个 Token 类型通过 chains 字段去区分在各链上的场景时,有两种实现:
- 组件就需要消费 Chain 字段,通过 Chain 字段去过滤出最终的 TokenList
- 外部调用者传递的对应链的 TokenList 中,需要将 chains 构造成数组类型,我这边组件去消费第 0 个
但从组件的设计来说,两者都不是好的实现,不论是 TokenSelect 组件还是 CryptoInput 组件,我消费的就是确定的 Token,不需要关注链的概念
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.
感觉没这么复杂,我们要做的就是定义一个 Token,确定 Token 有哪些属性。
那这个 token 是发行在哪个链上的,或者哪几个链上的就是这个 Token 的信息,就定义清楚就行。只是说看如何定义更清晰,用起来更方便。
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.
至于组件内部,循环遍历判断取一下要的信息我觉得还好,没啥问题,复杂度也还好。这个 Token 定义的信息以后可能别的地方也会用到,所以重点还是从这个 Token 本身要定义的内容来看,消费的地方要怎么消费就怎么用。
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.
至于组件内部,循环遍历判断取一下要的信息我觉得还好,没啥问题,复杂度也还好。这个 Token 定义的信息以后可能别的地方也会用到,所以重点还是从这个 Token 本身要定义的内容来看,消费的地方要怎么消费就怎么用。
OK 那先合并好了
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.
调整好了
packages/common/src/types.ts
Outdated
@@ -37,6 +37,7 @@ export type BalanceMetadata = { | |||
export interface Chain { | |||
id: ChainIds | number; | |||
name: string; | |||
type?: 'EVM' | 'JVM' | 'SVM' | 'WASM'; |
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.
感觉应该这样分,就是分对应我们现在支持的三种适配器,也是对应三种不同的链,wasm 好像没有什么意义:
export enum ChainType {
Ethereum = 'eth',
Solona = 'sol',
Bitcoin = 'btc',
}
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.
感觉应该这样分,就是分对应我们现在支持的三种适配器,也是对应三种不同的链,wasm 好像没有什么意义:
export enum ChainType { Ethereum = 'eth', Solona = 'sol', Bitcoin = 'btc', }
这个地方是按照虚拟机类型去分的,以太坊虚拟机兼容链、JAVA 虚拟机(Cadano)、Solana 虚拟机、WASM 虚拟机
如果要适配我们当前有的类型的话,是不是改成 EVM、SVM 和 Bitcoin
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.
嗯,可以,那就 EVM、SVM 和 Bitcoin。
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.
嗯,可以,那就 EVM、SVM 和 Bitcoin。
done
.changeset/brown-eyes-deny.md
Outdated
@@ -0,0 +1,5 @@ | |||
--- | |||
'@ant-design/web3-common': patch |
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.
新的 feature 应该用 minor
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.
新的 feature 应该用 minor
OK 之前是觉得只是类型的新增 没有新增具体的组件 就设置成 patch
@MarioJames , 下次可以用squash merge,更整洁干净。 |
好的 下次一定注意 |
[中文版模板 / Chinese template]
💡 Background and solution
新增 Token 基本类型,基于基本类型拓展出单链及多链 Token 类型,用于 TokenSelect/CryptoInput/PayPanel 组件
add TokenBase type, extends ChainToken and MultiChainToken type, suit for TokenSelect/CryptoInput/PayPanel Component
🔗 Related Pull Request link
TokenSelect/CryptoInput
PayPanel