Skip to content

CSSStyleDeclaration string properties all have | null type declaration. #13644

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

Closed
sccolbert opened this issue Jan 23, 2017 · 7 comments
Closed
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Suggestion An idea for TypeScript

Comments

@sccolbert
Copy link

lib.dom.d.ts has recently suffixed all of the properties of CSSStyleDeclaration with | null:
https://github.com/Microsoft/TypeScript/blob/master/lib/lib.dom.d.ts#L1382
6814c1d

For those of us using --strictNullChecks, this is forcing a null assertion (!) every time a property is used, even though according to the spec null will never be returned, only an empty string:
https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-getpropertyvalue

The camel-cased attribute attribute, on getting, must return the result of invoking getPropertyValue() with the argument being the result of running the IDL attribute to CSS property algorithm for camel-cased attribute.

The spec does seem to allow null as an argument for setPropertyValue, so technically this a case where allowing a different type for a getter and setter (#2521) would be required to accurately model a builtin.

But in the absence of that, this particular object treats the empty string the same as null for setting, so I think it would be cleaner to simply remove | null from this definition so that we don't have to needlessly null-assert everywhere CSSStyleDeclaration is used.

@mhegazy mhegazy added Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Apr 24, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Apr 24, 2017

Since this is strictly not per spec, we would like to get some more user feedback to warrant the change.

@inad9300
Copy link

Just as a curiosity, why were the nulls added in the first place? What was the feedback then?

@kitsonk
Copy link
Contributor

kitsonk commented Aug 16, 2017

The libraries are generated from a WebIDL, with overrides. I believe it is still the WebIDL from Edge, though there was a desire to generate it from the spec.

There is seldom any feedback required to make something more compliant with the spec and in many cases it is automatic as the upstream "authority" gets updated. The generator is available at Microsoft/TSJS-lib-generator, which includes the current version of the spec files it uses.

@inad9300
Copy link

It's very interesting to know this, thanks! I didn't know such level of automation was in place. This is then definitely related with the issue I just opened about missing HTML tags.

@HolgerJeromin
Copy link
Contributor

Currently we have mixed string and string | null typings 8-/
https://github.com/Microsoft/TypeScript/blob/master/lib/lib.dom.d.ts#L2749

@ypresto
Copy link
Contributor

ypresto commented Dec 26, 2019

Perhaps already fixed around here?:
microsoft/TypeScript-DOM-lib-generator#723

@amakhrov
Copy link

amakhrov commented Apr 9, 2020

Looks like this has landed in TS 3.6.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

8 participants