chore: remove ts-nocheck comments in select-v2 #16746
Conversation
|
馃憢 @makedopamine, thank you for contributing element-plus.
|
|
|
|
Hello @makedopamine, thank you for contributing to element-plus, please see our guideline to see how to make contribution |
github-actions
bot
added
Contribution::Community
CommitMessage::Qualified
|
馃И Playground Preview: https://element-plus.run/?pr=16746 |
|
Found hundreds of |
I actually notice the problem what you say, however, its workload is somewhat heavy to address it completely in a single PR. And I think a large PR is hard for reviewers to review, so I prefer to split the task into separate PRs. |
|
|
||
| export const SelectProps = buildProps({ | ||
| export const selectProps = buildProps({ |
There was a problem hiding this comment.
It may be better to keep the first letter capitalized here, otherwise it will be a breaking change for users who use related values.
There was a problem hiding this comment.
You are right.
| @@ -248,7 +255,7 @@ export const SelectProps = buildProps({ | |||
| ...useAriaProps(['ariaLabel']), | |||
| } as const) | |||
|
|
|||
| export const OptionProps = buildProps({ | |||
| export const optionProps = buildProps({ | |||
| @@ -190,7 +220,7 @@ export default defineComponent({ | |||
| } | |||
|
|
|||
| const onEscOrTab = () => { | |||
| select.expanded = false | |||
| select.expanded.value = false | |||
There was a problem hiding this comment.
Yep, it should indeed not be modified in this PR, causing confusion. I will revert it.
| @@ -1,3 +1,5 @@ | |||
| import type { Nullable } from '@element-plus/utils' | |||
There was a problem hiding this comment.
Maybe Nullable should no longer be used here, as it will be deprecated later.
| expanded: boolean | ||
| tooltipRef: Ref<TooltipInstance> | ||
| props: ExtractPropTypes<typeof selectProps> | ||
| expanded: Ref<boolean> |
There was a problem hiding this comment.
Why has the type changed here?
There was a problem hiding this comment.
For select.expanded.value = false
| @@ -18,7 +18,7 @@ export const defaultProps: Required<Props> = { | |||
| options: 'options', | |||
| } | |||
|
|
|||
| export function useProps(props: Pick<ISelectV2Props, 'props'>) { | |||
| export function useProps(props: ISelectV2Props) { | |||
There was a problem hiding this comment.
It seems that there is no need to change here. Only props.props is used below, and only the corresponding types are extracted.
There was a problem hiding this comment.
I think the type should match the runtime value. Anyway, it doesn't matter. I'm fine with both.
| import type { ISelectV2Props } from './token' | ||
| import type { SelectEmitFn } from './defaults' | ||
| import type { TooltipInstance } from '@element-plus/components/tooltip' | ||
| import type { Nullable } from '@element-plus/utils/typescript' |
There was a problem hiding this comment.
Nullable should no longer be used.
|
Thanks for your review. There's been a modification, please take a look. |
|
I pushed a new commit, kindly take note. |
makedopamine
force-pushed
the
chore/selectv2_remove_tsnocheck
branch
from
b10e785
to
31abd48
Compare
makedopamine
force-pushed
the
chore/selectv2_remove_tsnocheck
branch
from
76950f3
to
595ea25
Compare
No description provided.