-
-
Notifications
You must be signed in to change notification settings - Fork 9k
types(jsx): improve autocomplete type #14237
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds multiple TypeScript type aliases describing HTML autocomplete/autofill tokens in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
packages/runtime-dom/src/jsx.ts
Outdated
| export type AutoFillAddressKind = 'billing' | 'shipping' | ||
| export type AutoFillBase = '' | 'off' | 'on' | ||
| export type AutoFillContactField = | ||
| | 'email' | ||
| | 'tel' | ||
| | 'tel-area-code' | ||
| | 'tel-country-code' | ||
| | 'tel-extension' | ||
| | 'tel-local' | ||
| | 'tel-local-prefix' | ||
| | 'tel-local-suffix' | ||
| | 'tel-national' | ||
| export type AutoFillContactKind = 'home' | 'mobile' | 'work' | ||
| export type AutoFillCredentialField = 'webauthn' | ||
| export type AutoFillNormalField = | ||
| | 'additional-name' | ||
| | 'address-level1' | ||
| | 'address-level2' | ||
| | 'address-level3' | ||
| | 'address-level4' | ||
| | 'address-line1' | ||
| | 'address-line2' | ||
| | 'address-line3' | ||
| | 'bday-day' | ||
| | 'bday-month' | ||
| | 'bday-year' | ||
| | 'cc-csc' | ||
| | 'cc-exp' | ||
| | 'cc-exp-month' | ||
| | 'cc-exp-year' | ||
| | 'cc-family-name' | ||
| | 'cc-given-name' | ||
| | 'cc-name' | ||
| | 'cc-number' | ||
| | 'cc-type' | ||
| | 'country' | ||
| | 'country-name' | ||
| | 'current-password' | ||
| | 'family-name' | ||
| | 'given-name' | ||
| | 'honorific-prefix' | ||
| | 'honorific-suffix' | ||
| | 'name' | ||
| | 'new-password' | ||
| | 'one-time-code' | ||
| | 'organization' | ||
| | 'postal-code' | ||
| | 'street-address' | ||
| | 'transaction-amount' | ||
| | 'transaction-currency' | ||
| | 'username' | ||
| export type OptionalPrefixToken<T extends string> = `${T} ` | '' | ||
| export type OptionalPostfixToken<T extends string> = ` ${T}` | '' | ||
| export type AutoFillField = | ||
| | AutoFillNormalField | ||
| | `${OptionalPrefixToken<AutoFillContactKind>}${AutoFillContactField}` | ||
| export type AutoFillSection = `section-${string}` | ||
| export type AutoFill = | ||
| | AutoFillBase | ||
| | `${OptionalPrefixToken<AutoFillSection>}${OptionalPrefixToken<AutoFillAddressKind>}${AutoFillField}${OptionalPostfixToken<AutoFillCredentialField>}` |
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.
I don't think these types should be exported.
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.
Done, Thanks for your review!
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.
InputAutoCompleteAttribute can be exported like the InputTypeHTMLAttribute above.
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.
Okay, I didn't notice.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/runtime-dom/src/jsx.ts (1)
626-629: LGTM!The updated type provides better autocomplete suggestions while maintaining backward compatibility.
For consistency, consider applying
InputAutoCompleteAttributeto other elements that support theautocompleteattribute in a follow-up PR:
FormHTMLAttributes.autocomplete(line 484)SelectHTMLAttributes.autocomplete(line 799)TextareaHTMLAttributes.autocomplete(line 833)These currently use
string | undefined.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/runtime-dom/src/jsx.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test / e2e-test
🔇 Additional comments (1)
packages/runtime-dom/src/jsx.ts (1)
564-624: Well-structured autocomplete type definitions following established patterns.The type definitions correctly model the HTML autofill field names with composable template literal types. The approach mirrors the existing
InputTypeHTMLAttributepattern on lines 539-562, and the(string & {})escape hatch ensures backward compatibility while providing autocomplete for known values.Only exporting
InputAutoCompleteAttributewhile keeping intermediate types internal is the right call, as per the previous review feedback.
The code is sourced from DefinitelyTyped
Summary by CodeRabbit
autocompleteattributes, offering richer, more specific autocomplete options and stronger typing for billing, shipping, contact, and credential-related form fields — enhancing IDE suggestions and reducing form-related type errors.✏️ Tip: You can customize this high-level summary in your review settings.