-
-
Notifications
You must be signed in to change notification settings - Fork 7k
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
docs: updated missing descriptions on API #21160
base: master
Are you sure you want to change the base?
Conversation
Filled out documentation on component json files. Fixed type inference on exposed props for VTextField, consequently fixing missing descriptions for VAutocomplete, VCombobox, VDateInput, and VSelect.
vTextFieldRef.value?.setSelectionRange(0, 0) | ||
} | ||
} | ||
|
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.
What does this have to do with documentation?
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.
- Automplete was not rendering its exposed prop because
vTextFieldRef
did not have type. - After type was added. Cognitive complexity was triggered. Hence the need for the refactor.
@@ -25,6 +26,9 @@ | |||
"item.data-table-expand": "Slot to replace the default `v-icon` used when expanding rows.", | |||
"item.data-table-select": "Slot to replace the default checkbox used when selecting rows.", | |||
"loading": "Defines content for when `loading` is true and no items are provided.", | |||
"tbody": "Slot to replace the default table `<tbody>`.", | |||
"thead": "Slot to replace the default table `<thead>`.", | |||
"tfoot": "Slot to replace the default table `<tfoot>`.", |
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.
These slots have misleading names, tbody and tfoot are after the default tbody and thead is between the default thead and tbody.
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.
on second look, maybe these slots doc are not required for VDataTableVirtual
API docs. I have to double check why it was shouting missing when I was generating the api.
Anyways, could your comment be applicable to the current doc for VDataTable
?
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.
Yeah that's wrong too, if it replaced the default it would be slots.tbody?.() ?? (<tbody>
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.
Thanks, I'll update VDataTable
current doc too. Slot after the default...
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.
these slots are meant to used with the hide-default-(body|footer|header)
props to replace the slot, which should be worth mentioning in the descriptions.
@@ -292,4 +292,5 @@ export const VTextField = genericComponent<VTextFieldSlots>()({ | |||
}, | |||
}) | |||
|
|||
export type VTextField = InstanceType<typeof VTextField> | |||
// Omit HTMLInputElement prop to prevent inference when generating docs | |||
export type VTextField = Omit<InstanceType<typeof VTextField>, keyof HTMLInputElement> |
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.
This is not a good solution, those properties are actually accessible on the instance and should be present in the public types. If something is causing incorrect inference it should be fixed in
vuetify/packages/api-generator/templates/component.d.ts
Lines 49 to 54 in 3fde52a
type ExtractExposed<T> = T extends ComponentOptionsBase<any, infer B, any, any, any, any, any, any> | |
? B extends void ? never | |
: B extends { _allExposed: infer E } ? E | |
: B extends object ? B | |
: never | |
: never |
vuetify/packages/vuetify/src/composables/forwardRefs.ts
Lines 29 to 41 in 42dba04
T extends {}, | |
U extends Ref<HTMLElement | Omit<ComponentPublicInstance, '$emit' | '$slots'> | undefined>[], | |
UU = { [K in keyof U]: NonNullable<UnwrapRef<U[K]>> }[number], | |
UC = { [K in keyof U]: OmitPrivate<OmitProps<NonNullable<UnwrapRef<U[K]>>>> }[number], | |
R = T & UnionToIntersection<UC> & { | |
_allExposed: T | ( | |
UU extends { $options: infer O } | |
? O extends ComponentOptionsBase<any, infer E, any, any, any, any, any, any> | |
? E | |
: never | |
: never | |
) | |
} |
vuetify/packages/api-generator/src/types.ts
Lines 412 to 568 in 44be603
function generateDefinition (node: Node<ts.Node>, recursed: string[], project: Project, type?: Type<ts.Type>): Definition { | |
const tc = project.getTypeChecker() | |
type = type ?? node.getType() | |
if (type.getAliasSymbol()?.getName() === 'NonNullable') { | |
const typeArguments = type.getAliasTypeArguments() | |
if (typeArguments.length) { | |
type = typeArguments[0] | |
} | |
} | |
const symbol = type.getAliasSymbol() ?? type.getSymbol() | |
const declaration = symbol?.getDeclarations()?.[0] | |
const targetType = type.getTargetType() | |
let definition: Definition = { | |
text: getCleanText(type.getText()), | |
source: getSource(declaration), | |
} as Definition | |
if ( | |
count(recursed, type.getText()) > 1 || | |
allowedRefs.includes(symbol?.getName() as string) || | |
isExternalDeclaration(declaration, definition.text) | |
) { | |
definition = definition as RefDefinition | |
definition.type = 'ref' | |
// TODO: Parse this better? | |
definition.ref = symbol?.getFullyQualifiedName().replace(/".*"\./, '') ?? '' | |
} else if (type.isAny() || type.isUnknown() || type.isNever()) { | |
// @ts-expect-error asd | |
definition.type = type.getText() | |
} else if (type.isBoolean() || type.isBooleanLiteral()) { | |
definition = definition as BooleanDefinition | |
definition.type = 'boolean' | |
definition.literal = type.isBooleanLiteral() ? type.getText() : undefined | |
} else if (type.isString() || type.isStringLiteral()) { | |
definition = definition as StringDefinition | |
definition.type = 'string' | |
definition.literal = type.isStringLiteral() ? type.getText() : undefined | |
} else if (type.isNumber() || type.isNumberLiteral()) { | |
definition = definition as NumberDefinition | |
definition.type = 'number' | |
definition.literal = type.isNumberLiteral() ? type.getText() : undefined | |
} else if (type.isArray()) { | |
definition = definition as ArrayDefinition | |
definition.type = 'array' | |
const arrayElementType = type.getArrayElementType() | |
const arrayType = generateDefinition(node, getRecursiveTypes(recursed, type), project, arrayElementType) | |
definition.items = arrayType.type === 'anyOf' ? arrayType.items : [arrayType] | |
} else if (type.isIntersection()) { | |
definition = definition as IntersectionDefinition | |
definition.type = 'allOf' | |
definition.items = type.getIntersectionTypes() | |
.map(intersectionType => generateDefinition(node, recursed, project, intersectionType)) | |
// TODO: Should we collapse allOf with only objects to single object? | |
} else if (type.isUnion()) { | |
definition = definition as UnionDefinition | |
definition.type = 'anyOf' | |
definition.items = getUnionTypes(type) | |
.map(unionType => generateDefinition(node, recursed, project, unionType)) | |
// Replace explicit true|false with boolean | |
// TODO: Do this some other way | |
let found = -1 | |
for (let i = 0; i < definition.items.length; i++) { | |
const item = definition.items[i] | |
if (item.type === 'boolean' && item.literal != null) { | |
if (~found) { | |
definition.items.splice(i, 1) | |
definition.items.splice(found, 1, { | |
text: 'boolean', | |
type: 'boolean', | |
formatted: 'boolean', | |
}) | |
break | |
} else { | |
found = i | |
} | |
} | |
} | |
} else if (type.getConstructSignatures().length) { | |
definition = definition as ConstructorDefinition | |
definition.type = 'constructor' | |
} else if (type.getCallSignatures().length) { | |
definition = definition as FunctionDefinition | |
definition.type = 'function' | |
const signature = type.getCallSignatures()[0] | |
definition.parameters = signature.getParameters().map(parameter => { | |
const parameterType = tc.getTypeOfSymbolAtLocation(parameter, node) | |
return { | |
name: parameter.getEscapedName(), | |
optional: parameter.isOptional(), | |
...generateDefinition(node, getRecursiveTypes(recursed, parameterType), project, parameterType), | |
} | |
}) | |
const returnType = signature.getReturnType() | |
definition.returnType = generateDefinition(node, getRecursiveTypes(recursed, returnType), project, returnType) | |
} else if (targetType && /^(Map|Set)<.*>/.test(definition.text)) { // TODO: Better way to detect Map/Set type | |
definition = definition as InterfaceDefinition | |
definition.type = 'interface' | |
definition.name = targetType?.getText() | |
const instanceTypeArguments = type.getTypeArguments() | |
definition.parameters = targetType?.getTypeArguments().map((arg, i) => { | |
return { name: arg.getText(), ...generateDefinition(node, recursed, project, instanceTypeArguments[i]) } | |
}) | |
} else if (/^Record<.*>/.test(definition.text)) { // TODO: Better way to detect Record type | |
definition = definition as RecordDefinition | |
definition.type = 'record' | |
definition.key = generateDefinition(node, recursed, project, type.getAliasTypeArguments()[0]) | |
definition.value = generateDefinition(node, recursed, project, type.getAliasTypeArguments()[1]) | |
} else if (type.isTuple()) { | |
definition = definition as ArrayDefinition | |
definition.type = 'array' | |
definition.items = type.getTupleElements().map(t => generateDefinition(node, recursed, project, t)) | |
definition.length = definition.items.length | |
} else if (type.isObject()) { | |
definition = definition as ObjectDefinition | |
definition.type = 'object' | |
definition.properties = {} | |
for (const property of type.getProperties()) { | |
const propertyName = property.getEscapedName() | |
const propertyType = tc.getTypeOfSymbolAtLocation(property, node) | |
definition.properties[propertyName] = generateDefinition(node, getRecursiveTypes(recursed, propertyType), project, propertyType) | |
definition.properties[propertyName].optional = property.isOptional() | |
} | |
if (type.compilerType.indexInfos.length) { | |
for (const index of type.compilerType.indexInfos) { | |
const indexName = '[' + type._context.compilerFactory.getType(index.keyType).getText() + ']' | |
const indexType = type._context.compilerFactory.getType(index.type) | |
definition.properties[indexName] = generateDefinition(node, getRecursiveTypes(recursed, indexType), project, indexType) | |
definition.properties[indexName].optional = true | |
} | |
} | |
} else if (ts.TypeFlags.Void & type.getFlags()) { | |
// @ts-expect-error asd | |
definition.type = 'void' | |
} else { | |
// @ts-expect-error asd | |
definition.type = 'UNSUPPORTED' | |
} | |
formatDefinition(definition) | |
return definition | |
} |
If you have absolutely no idea wtf is going on in there (don't blame you) feel free to ask for a hand.
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.
yeah I don't like it either. I added <TextField & HTMLInputElement>
as type on all components that use the TextField
instance as a workaround. But thanks I'll look into this, I think there's an underlying recursion error in generating the docs that must be its own PR.
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.
Just saw the last line of your comment 😅 . Yes I was in this file trying to figure out how I can correctly infer HTMLInputElement
. VTextField
is inferring it fine anyway 🤷♂️ . But when VTextField
is a ref then there's the issue. I'll defer the work to another PR if it's okay with you. Then merge it back here.
Description
HTMLInputElement
fromVTextField
type to fix missing descriptions onVAutocomplete
,VCombobox
,VDateInput
, andVSelect
. Workaround until doc generation recursion issue is fixed.VTextField
ref and requiringHTMLInputElement
props, use<VTextField & HTMLInputElement>
as Ref typeVTimePickerControls
correct type forperiod
prop.Markup:
N/A