docs: WIP create a more strongly typed schema for emits #2741
Conversation
|
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
commit: |
Let me try to paraphrase to make sure I'm understanding. Should we do the same thing with emits and slots as you did with props, where the component data in the docs is an object that is type-checked against the actual component's type rather than an array, even if we can't type check the args? I'd say yes. I'm curious by what you mean by bulky? Adding the typing to each emeit section in the docs? I'd definitely take that for the benefit of keeping the docs more in sync with the source. |
This: const boundBTableLiteEmits = {
onHeadClicked: (fieldKey, field, event, isFooter = false) => {
emit('head-clicked', fieldKey, field, event, isFooter)
handleFieldSorting(field)
},
onRowClicked: (row, index, e) => {
if (props.noSelectOnClick === false) {
handleRowSelection(row, index, e.shiftKey, e.ctrlKey, e.metaKey)
}
emit('row-clicked', row, index, e)
},
onRowDblclicked: (...args) => emit('row-dblclicked', ...args),
onRowContextmenu: (...args) => emit('row-contextmenu', ...args),
onRowHovered: (...args) => emit('row-hovered', ...args),
onRowUnhovered: (...args) => emit('row-unhovered', ...args),
onRowMiddleClicked: (...args) => emit('row-middle-clicked', ...args),
} as const satisfies {
[K in keyof BTableLiteEmits<Items, FieldsType> as CamelCase<`on-${K & string}`>]: (
...args: BTableLiteEmits<Items, FieldsType>[K]
) => void
} |
I see it does feel a bit bulky @ 20ish lines of code. But doesn't that code replace lines 5-32 (28 lines)+ the definitions of onRowClick (7 lines) and onFieldHeadClick (10 lines) = 45 lines? Seems like an improvement to me. |
|
@VividLemon This seems fine. Passing the emits is annoying anyway, so I'm for more type safety. |
@xvaara I'm curious on what you think about the way I handled passthrough of emits to btablesimple here. Do you think it should be applied for more components that inherit emits from others? It's a bit bulky to convert from it's emit, to include 'on-' then to camelize it (all in types), then pass through as a v-bind. But it provides additional type safety in adding new emits in sub components.
@dwgray I'm also curious what you think in the benefit shown to the documentation. It allows you to get the name of the emit, but provides no additional benefit for the arguments in that emit (as they are a tuple and can't get the named values in a tuple). It's also pretty unlike the already made 'BvnComponentEmits', as it just does
export type * as BvnComponentEmits. Ideally the schema would be like{ [componentName]: { props: {}, emits: {} }to which you could doBvnComponent['BAccordion']['emit'], to keep everything all tidy together, but this is also quite bulky to doDoes this seem worthwhile? Should slots follow in addition for their inheritance? Originally the thought process was to stay away from inheritance, but it seems like these three could benefit from it.