Skip to content
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.

Sign up for GitHub

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

refactor(components): [backTop] refactor #5371

Closed
wants to merge 1 commit into from
Closed

refactor(components): [backTop] refactor #5371

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion components/back-top/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,20 @@ export const backTopProps = {
// visible: PropTypes.looseBool, // Only for test. Don't use it.
};

export const backTopEmits = {
click: (e: Event) => e instanceof Event,
};
export type BackTopEmits = typeof backTopEmits;

export type BackTopInstance = InstanceType<typeof BackTop>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个主要用来做什么用呢

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些用到的时候自然有用，例如：通过ref调用组件实例方法的时候

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个我们只需要暴露组件内 expose 出去的方法就好了，其他方法都是不安全的，不应该暴露给用户。
如果非要用，在用户层面自己 InstanceType 好了

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK


export type BackTopProps = Partial<ExtractPropTypes<typeof backTopProps>>;

const BackTop = defineComponent({
name: 'ABackTop',
inheritAttrs: false,
props: backTopProps,
emits: ['click'],
emits: backTopEmits,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我们计划去掉 emits 声明，全都放入 props 中去声明

Copy link
Member Author

@buqiyuan buqiyuan Mar 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔全部放props里面？这样不好吧，vue不同于react，react所有的属性、事件回调、children(插槽)都是通过props传递，但在vue中是将react的props拆分为了props + emits + slots相对独立的3部分，虽然在vue中也完全可以像react那样，将所有东西都通过props进行传递，但两者对props的处理还是略有不同的。我觉得咱们应该遵循vue的设计理念，props和emits应该是分开来的，自定义事件都应该定义在emits中，如果都定义在props中，在对antdv组件进行二次封装的时候，如果把antdv组件原有的props解构到封装的组件props上的话，再将当前组件的props透传给antdv组件的时候，我需要手动剔除一些属性，遇到的一些小问题，可以看下我这里留下的评论

Copy link
Member Author

@buqiyuan buqiyuan Mar 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

在之前issue中也能找到一些相关的问题，但是在setup-script语法中，即使通过defineEmits定义了，但这些事件仍会被合并成一个数组

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我觉得合并数组的问题可以尝试给 vue 提PR，添加配置项
还有一点是定义在 emits 中有几个小问题：
1、无法获取用户是否传递了某些事件，有些组件会根据是否传递有不同的行为，vue 未来可能会支持
2、无法获取事件返回值，有些事件需要返回值，如 true、false、promise 等，过去我们单独处理了这块

如果想统一掉，props 是一个合适(唯一)的选择

PS：在 vue2中，props + emits + slots相对独立，在 vue3 中已经渐渐淡化这个概念了，而且 slots 未来有可能也会有定义在 props 中的语法糖：vuejs/rfcs#192 (comment)

Copy link
Member Author

@buqiyuan buqiyuan Mar 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

关于您提到的两点：
1、可以通过getCurrentInstance()?.vnode?.props拿到用户是否传递了某些事件(emit)，这里的getCurrentInstance()?.vnode?.props其实就是完全等同于react的props了，也就是你现在定义在props里面的所有东西，这里vnode的props是包含了component的props + attrs的。
2、自定义事件(emit)我认为是不需要返回值的，因为父组件仅仅是监听了子组件内部触发的事件并接收了回调参数而已，需要返回值的通过props传递一个回调函数即可，自定义事件(emit)和回调函数(callback)应该是要区分开来的。

PS：在vue3中，以前vue2中适用的概念确实被淡化了不少，但我觉得slots被定义在props这个可能性不大，而且之前vue与ts存在的类型系统断层问题，现在通过工具链的支持已经变得很好了，即是说vue与ts相性不好的问题通过工具链的桥接一样可以做到react + ts的体验。
image
image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getCurrentInstance()?.vnode?.props 算是一个非文档 API(黑科技)，随时都有可能会改动，antdv1.x 就是用了很多黑科技，导致升级很困难，我们应该尽可能避免这类使用，虽然现在还有一些”黑科技“，但一直在慢慢偿还债务。

对于组件库来说，统一规范定义在props，可以减少很多工作量。
而对于普通用户来说，是无感知的。
对于二开用户或二次封装用户，这都不是事。

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

对于非文档中公开的API(黑科技)的使用确实需要考量，行！那我明白了。然后大概看了下目录源码，感觉还可以精简很多

setup(props, { slots, attrs, emit }) {
const { prefixCls, direction } = useConfigInject('back-top', props);
const domRef = ref();
Expand Down