microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.08k stars 12.37k forks source link

Improve CSSStyleDeclaration typings #17827

Open inad9300 opened 7 years ago

inad9300 commented 7 years ago

Not every string should be allowed in properties of the CSSStyleDeclaration interface, such as display, overflow, etc. The TypeStyle project seems to have put quite some effort into this already, from which inspiration could be taken.

inad9300 commented 6 years ago

I've just been told about https://github.com/frenic/csstype and wanted to share it with you as well since it could be of help in resolving this issue.

edmundmaruhn commented 6 years ago

Just a question: Why is the index type of CSSStyleDeclaration's index signature number and not string?

In case you want to apply a list of styles to HTMLElement.style one (like me) could try:

let element: HTMLElement = window.document.create('div');
let styles: { [key: string]: string } = {
    display: 'inline-block',
    width: '36px',
    height: '36px'
};

for (let style in styles) {
    element.style[style] = styles[style]; // generates TS7015, because index type is not 'number'
}

My current workaround is a cast to any:

(<any>element.style)[style] = styles[style];

which is really ugly.

jccr commented 4 years ago

It appears that somebody contributed a potential fix: https://github.com/microsoft/TypeScript/pull/35107

The bot declined it:

It looks like you've sent a pull request to update some generated declaration files related to the DOM. These files aren't meant to be edited by hand, as they are synchronized with files in the TSJS-lib-generator repository. You can read more here. For house-keeping purposes, this pull request will be closed.

And after a quick look into the TSJS-lib-generator repo... it looks like no changes related to this have happened there yet:

https://github.com/microsoft/TSJS-lib-generator/blob/master/baselines/dom.generated.d.ts#L3196

I would like to add to the opinion that this is a real issue that needs to be addressed.

rpearce commented 4 years ago

Is there any special reason this line is has index: number and not a index: string?

https://github.com/microsoft/TSJS-lib-generator/blob/bf671e99615b06404db7a89fa9ae05bf14a8e47c/baselines/dom.generated.d.ts#L3201

inad9300 commented 4 years ago

@rpearce this seems to be the culprit: https://developer.mozilla.org/en-US/docs/Web/API/CSSStyleDeclaration/item

rpearce commented 4 years ago

Wow, today I learned! Thank you.

GlenPerkins commented 3 years ago

"ts(7015)". TypeScript is still insisting that standard CSS style names are numbers like 1, 2, 3, not strings like 'margin' and 'font-size', so mydiv.style['font-size'] is a type error that I need to fix by either conforming to the standard: mydiv.style[37] = '24px' or working around it and living on the edge.

HolgerJeromin commented 3 years ago

@GlenPerkins Just use: mydiv.style.setProperty('font-size', '24px') and mydiv.style.getPropertyValue('font-size')

GlenPerkins commented 3 years ago

@HolgerJeromin Thanks. I still think that TypeScript shouldn't be flagging the standard style['font-size'] as an error, but I appreciate your reminder of the set/get method approach, and I'll just use that instead. It's as standard as the other, and TypeScript doesn't think it's wrong.

kaisnb commented 5 months ago

@GlenPerkins Instead of style['font-size'] you can use style.fontSize. In case the key is a variable we can make it typesafe like so:

// By using Omit I remove all functions and readonly properties from the CSSStyleDeclaration
// type and by using Excluse I remove the number index.
export type StyleProperty = Exclude<
  keyof Omit<
    CSSStyleDeclaration,
    | 'length'
    | 'parentRule'
    | 'getPropertyPriority'
    | 'getPropertyValue'
    | 'item'
    | 'removeProperty'
    | 'setProperty'
  >,
  number
>;
// Now we can use this type to make index access of "style" typesafe:
const styleProperty: StyleProperty = 'fontSize';
style[styleProperty] = '14px';