Skip to content

Add more precise typing for autocomplete HTML attribute #1467

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

Merged
Merged
19 changes: 15 additions & 4 deletions baselines/dom.generated.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10260,7 +10260,7 @@ interface HTMLFormElement extends HTMLElement {
*
* [MDN Reference](https://developer.mozilla.org/docs/Web/API/HTMLFormElement/autocomplete)
*/
autocomplete: string;
autocomplete: AutoFillBase;
/**
* Retrieves a collection, in source order, of all controls in a given form.
*
Expand Down Expand Up @@ -10928,7 +10928,7 @@ interface HTMLInputElement extends HTMLElement, PopoverInvokerElement {
*
* [MDN Reference](https://developer.mozilla.org/docs/Web/API/HTMLInputElement/autocomplete)
*/
autocomplete: string;
autocomplete: AutoFill;
capture: string;
/** Sets or retrieves the state of the check box or radio button. */
checked: boolean;
Expand Down Expand Up @@ -12309,7 +12309,7 @@ declare var HTMLScriptElement: {
*/
interface HTMLSelectElement extends HTMLElement {
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/HTMLSelectElement/autocomplete) */
autocomplete: string;
autocomplete: AutoFill;
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/HTMLSelectElement/disabled) */
disabled: boolean;
/**
Expand Down Expand Up @@ -13111,7 +13111,7 @@ declare var HTMLTemplateElement: {
*/
interface HTMLTextAreaElement extends HTMLElement {
/** [MDN Reference](https://developer.mozilla.org/docs/Web/API/HTMLTextAreaElement/autocomplete) */
autocomplete: string;
autocomplete: AutoFill;
/** Sets or retrieves the width of the object. */
cols: number;
/** Sets or retrieves the initial contents of the object. */
Expand Down Expand Up @@ -27926,6 +27926,9 @@ declare function addEventListener(type: string, listener: EventListenerOrEventLi
declare function removeEventListener<K extends keyof WindowEventMap>(type: K, listener: (this: Window, ev: WindowEventMap[K]) => any, options?: boolean | EventListenerOptions): void;
declare function removeEventListener(type: string, listener: EventListenerOrEventListenerObject, options?: boolean | EventListenerOptions): void;
type AlgorithmIdentifier = Algorithm | string;
type AutoFill = AutoFillBase | `${OptionalPrefixToken<AutoFillSection>}${OptionalPrefixToken<AutoFillAddressKind>}${AutoFillField}${OptionalPostfixToken<AutoFillCredentialField>}`;
type AutoFillField = AutoFillNormalField | `${OptionalPrefixToken<AutoFillContactKind>}${AutoFillContactField}`;
type AutoFillSection = `section-${string}`;
type BigInteger = Uint8Array;
type BinaryData = ArrayBuffer | ArrayBufferView;
type BlobPart = BufferSource | Blob | string;
Expand Down Expand Up @@ -27976,6 +27979,8 @@ type NamedCurve = string;
type OffscreenRenderingContext = OffscreenCanvasRenderingContext2D | ImageBitmapRenderingContext | WebGLRenderingContext | WebGL2RenderingContext;
type OnBeforeUnloadEventHandler = OnBeforeUnloadEventHandlerNonNull | null;
type OnErrorEventHandler = OnErrorEventHandlerNonNull | null;
type OptionalPostfixToken<T extends string> = ` ${T}` | "";
type OptionalPrefixToken<T extends string> = `${T} ` | "";
type PerformanceEntryList = PerformanceEntry[];
type ReadableStreamController<T> = ReadableStreamDefaultController<T> | ReadableByteStreamController;
type ReadableStreamReadResult<T> = ReadableStreamReadValueResult<T> | ReadableStreamReadDoneResult<T>;
Expand All @@ -28000,6 +28005,12 @@ type AudioContextLatencyCategory = "balanced" | "interactive" | "playback";
type AudioContextState = "closed" | "running" | "suspended";
type AuthenticatorAttachment = "cross-platform" | "platform";
type AuthenticatorTransport = "ble" | "hybrid" | "internal" | "nfc" | "usb";
type AutoFillAddressKind = "billing" | "shipping";
type AutoFillBase = "" | "off" | "on";
type AutoFillContactField = "email" | "tel" | "tel-area-code" | "tel-country-code" | "tel-extension" | "tel-local" | "tel-local-prefix" | "tel-local-suffix" | "tel-national";
type AutoFillContactKind = "home" | "mobile" | "work";
type AutoFillCredentialField = "webauthn";
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";
type AutoKeyword = "auto";
type AutomationRate = "a-rate" | "k-rate";
type AvcBitstreamFormat = "annexb" | "avc";
Expand Down
128 changes: 128 additions & 0 deletions inputfiles/addedTypes.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,98 @@
// https://developer.mozilla.org/en-US/docs/Web/API/WebXR_Device_API#browser_compatibility
"xr-spatial-tracking"
]
},
"AutoFillBase": {
"name": "AutoFillBase",
"value": [
// Off
"off",
// Automatic
"on",
""
]
},
"AutoFillAddressKind": {
"name": "AutoFillAddressKind",
"value": [
"shipping",
"billing"
]
},
"AutoFillNormalField": {
"name": "AutoFillNormalField",
"value": [
"name",
"honorific-prefix",
"given-name",
"additional-name",
"family-name",
"honorific-suffix",

"username",
"new-password",
"current-password",
// Supported in iOS Safari too even though WPT tests
// for Safari currently fail as of 2023-06.
"one-time-code",

"organization",
"street-address",
"address-line1",
"address-line2",
"address-line3",
"address-level4",
"address-level3",
"address-level2",
"address-level1",
"country",
"country-name",
"postal-code",

"cc-name",
"cc-given-name",
"cc-family-name",
"cc-number",
"cc-exp",
"cc-exp-month",
"cc-exp-year",
"cc-csc",
"cc-type",
"transaction-currency",
"transaction-amount",

"bday-day",
"bday-month",
"bday-year"
]
},
"AutoFillContactKind": {
"name": "AutoFillContactKind",
"value": [
"home",
"work",
"mobile"
]
},
"AutoFillContactField": {
"name": "AutoFillContactField",
"value": [
"tel",
"tel-country-code",
"tel-national",
"tel-area-code",
"tel-local",
"tel-local-prefix",
"tel-local-suffix",
"tel-extension",
"email"
]
},
"AutoFillCredentialField": {
"name": "AutoFillCredentialField",
"value": [
"webauthn"
]
}
}
},
Expand Down Expand Up @@ -1422,6 +1514,42 @@
{
"name": "EventListenerOrEventListenerObject",
"overrideType": "EventListener | EventListenerObject"
},
{
"name": "OptionalPrefixToken",
"typeParameters": [
{
"name": "T extends string"
}
],
"overrideType": "`${T} ` | \"\""
},
{
"name": "OptionalPostfixToken",
"typeParameters": [
{
"name": "T extends string"
}
],
"overrideType": "` ${T}` | \"\""
},
{
"name": "AutoFillSection",
// Note: this will also eagerly match any invalid string
// after section- instead of stopping at the first whitespace.
// It should be something like /section-\S/ if it were supported.
"overrideType": "`section-${string}`"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is problematic as it allows space: section-foo bar baz is allowed as AutoFillSection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh ok, There was a test for section-wrong off, and even just section-foo bar baz is disallowed, but indeed something like section-foo bar baz username currently passes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly it doesn't seem trivial, I don't think I'm fluent enough in typescript to do this (or if it's possible at all).

I will add tests that show the expected and unexpected cases anyway.

I'll continue looking into it but I would argue that the current type (before this PR) allows much more invalid values, so maybe we could iterate on it? Or do you feel like it would be a blocker?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can see how to do it with a generic, but not inside a template literal type:

playground

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, pretty sure it's not currently possible. Anyway as I said, still much less false positives than before. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

@sandersn This one is the main reason I'm not approving, what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I think some overgeneration is OK as long as it's not rejecting values that are legal.
I do wonder about performance though. Let me look again at the generated size of the type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is problematic as it allows space: section-foo bar baz is allowed as AutoFillSection.

Note for other readers that it is not a full catch-all, you still need to add valid tokens to your attribute, so for example, section-foo bar baz username would pass but not section-foo bar baz alone, nor section-wrong off as shown in the tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do wonder about performance though. Let me look again at the generated size of the type.

@sandersn: Hi, I've pushed the final version, are you ok with the generated types?

https://github.com/microsoft/TypeScript-DOM-lib-generator/pull/1467/files#diff-d57c51d024219b9f37e6b42df127db25f700acbaade23ce1edcecda2853b995c

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a TODO comment about the over-match of section-{string}?

},
{
"name": "AutoFillField",
"overrideType": "AutoFillNormalField | `${OptionalPrefixToken<AutoFillContactKind>}${AutoFillContactField}`"
},
{
// See the full list of possible autofill values for the `autocomplete` attribute:
// https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#determine-a-field's-category
// Full spec at https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#autofill.
"name": "AutoFill",
"overrideType": "AutoFillBase | `${OptionalPrefixToken<AutoFillSection>}${OptionalPrefixToken<AutoFillAddressKind>}${AutoFillField}${OptionalPostfixToken<AutoFillCredentialField>}`"
}
]
}
Expand Down
11 changes: 11 additions & 0 deletions inputfiles/knownTypes.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,15 @@
"AesGcmParams",
"AesKeyAlgorithm",
"AesKeyGenParams",
"AutoFillBase",
"AutoFillSection",
"AutoFillAddressKind",
"AutoFillNormalField",
"AutoFillContactKind",
"AutoFillContactField",
"AutoFillField",
"AutoFillCredentialField",
"AutoFill",
"BigInteger",
"ClientQueryOptions",
"ClientTypes",
Expand All @@ -29,6 +38,8 @@
"Keyframe",
"MutationRecordType",
"NamedCurve",
"OptionalPrefixToken",
"OptionalPostfixToken",
"Pbkdf2Params",
"PropertyIndexedKeyframes",
"RsaHashedImportParams",
Expand Down
22 changes: 21 additions & 1 deletion inputfiles/overridingTypes.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -1281,6 +1281,10 @@
"HTMLSelectElement": {
"properties": {
"property": {
"autocomplete": {
"name": "autocomplete",
"overrideType": "AutoFill"
},
"selectedOptions": {
"name": "selectedOptions",
"overrideType": "HTMLCollectionOf<HTMLOptionElement>"
Expand Down Expand Up @@ -1684,6 +1688,10 @@
"HTMLInputElement": {
"properties": {
"property": {
"autocomplete": {
"name": "autocomplete",
"overrideType": "AutoFill"
},
"selectionDirection": {
"name": "selectionDirection",
"overrideType": "\"forward\" | \"backward\" | \"none\""
Expand Down Expand Up @@ -1806,6 +1814,10 @@
"HTMLTextAreaElement": {
"properties": {
"property": {
"autocomplete": {
"name": "autocomplete",
"overrideType": "AutoFill"
},
"labels": {
"name": "labels",
"overrideType": "NodeListOf<HTMLLabelElement>"
Expand Down Expand Up @@ -2443,7 +2455,15 @@
"overrideIndexSignatures": [
"[index: number]: Element",
"[name: string]: any"
]
],
"properties": {
"property": {
"autocomplete": {
"name": "autocomplete",
"overrideType": "AutoFillBase"
}
}
}
},
"Blob": {
"methods": {
Expand Down
59 changes: 59 additions & 0 deletions unittests/files/autocomplete.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// @ts-expect-error should be a string
document.body.getElementsByTagName("input")[0].autocomplete = false;
// @ts-expect-error wrong value for this attribute
document.body.getElementsByTagName("input")[0].autocomplete = "undefined";
// @ts-expect-error does not accept boolean attributes
document.body.getElementsByTagName("input")[0].autocomplete = "true";
// @ts-expect-error does not accept boolean attributes
document.body.getElementsByTagName("input")[0].autocomplete = "false";

// @ts-expect-error missing autofill token before webauthn
document.body.getElementsByTagName("input")[0].autocomplete = "webauthn";

// @ts-expect-error wrong order for webauthn token
document.body.getElementsByTagName("input")[0].autocomplete =
"webauthn username";

// @ts-expect-error wrong order for contact specifier
document.body.getElementsByTagName("input")[0].autocomplete = "tel mobile";

// @ts-expect-error off should be standalone
document.body.getElementsByTagName("input")[0].autocomplete =
"section-wrong off";

// @ts-expect-error on should be standalone
document.body.getElementsByTagName("input")[0].autocomplete = "on username";

// @ts-expect-error home, work or mobile are only for contact tokens
document.body.getElementsByTagName("input")[0].autocomplete = "mobile username";

// @ts-expect-error should probably be current-password or new-password
document.body.getElementsByTagName("input")[0].autocomplete = "password";

document.body.getElementsByTagName("input")[0].autocomplete = "";
document.body.getElementsByTagName("input")[0].autocomplete = "on";
document.body.getElementsByTagName("input")[0].autocomplete = "off";
document.body.getElementsByTagName("input")[0].autocomplete = "new-password";
document.body.getElementsByTagName("input")[0].autocomplete =
"current-password";
document.body.getElementsByTagName("input")[0].autocomplete = "one-time-code";

document.body.getElementsByTagName("input")[0].autocomplete =
"username webauthn";

document.body.getElementsByTagName("input")[0].autocomplete =
"shipping street-address";
document.body.getElementsByTagName("input")[0].autocomplete =
"section-custom shipping street-address";

document.body.getElementsByTagName("input")[0].autocomplete = "work email";
document.body.getElementsByTagName("input")[0].autocomplete =
"section-custom billing mobile tel";

// @ts-expect-error only on and off are available on a form
document.body.getElementsByTagName("form")[0].autocomplete = "new-password";

document.body.getElementsByTagName("form")[0].autocomplete = "off";

// @ts-expect-error autocomplete attribute is only for form elements
document.body.getElementsByTagName("p")[0].autocomplete = "off";