invalid css properties not being type-checked

[zanona]

Current behavior:

Whenever using an invalid css property such as colour: 'red' typescript won't complain about it.

To reproduce:

import {css} from '@emotion/core';
const style = css({colour:'red'});

Expected behavior:

I would expect that as it happens with 'csstype', it would catch the type error on the property name as:

import CSS from 'csstype';
const style:CSS.Properties = {colour: 'red'};

Type '{ colour: string; }' is not assignable to type 'Properties<string | 0>'. Object literal may only specify known properties, but 'colour' does not exist in type 'Properties<string | 0>'. Did you mean to write 'color'?

Environment information:

• react version: "^15.0.0 || ^16.0.0"
• emotion version: "^10.0.28"

[Andarist]

This is because we explicitly allow other properties: https://github.com/emotion-js/emotion/blob/fc119766b539f73546dc8e2863cca1439af3cb1c/packages/serialize/types/index.d.ts#L37-L40

I'm not entirely sure why though, most likely to support non-standard properties that are not covered by csstype.

[zanona]

Thanks, @Andarist. That's an interesting approach, I guess that if the user had the option to extend the interface to add non-standard properties, it could still benefit those situations while actively helping to reduce typos and errors? After all, if there's any non-standard property being defined, it's likely to be user input anyway?

Perhaps a generic would make sense?

interface CustomProps extends CSSObject {
   nonStandardProp: string;
}

const style = css<CustomProps>({
   color: 'red',
   nonStandardProp: 'key'
})

[Andarist]

Oh, I thought right now about an even more important reason why this is like it is. It allows writing things like:

css({
  ':hover,:focus': {
    color: 'hotpink'
  }
})

or just

css({
  '[data-component="Link"]': {
    color: 'hotpink'
  }
})

[zanona]

Ah, yes. That makes sense :sunglasses: In this case I guess it would be tricky to account for all variations, unless a structure like this takes place one day: https://basarat.gitbook.io/typescript/type-system/index-signatures#design-pattern-nested-index-signature (very interesting and contextual section of the book)

interface NestedCSS {
  color?: string;
  nest?: {
    [selector: string]: NestedCSS;
  }
}

const example: NestedCSS = {
  color: 'red',
  nest: {
    '.subclass': {
      color: 'blue'
    }
  }
}

const failsSilently: NestedCSS = {
  colour: 'red', // TS Error: unknown property `colour`
}

~That'd be a huge breaking change though~ (unless it's added as an alternative accepted style, which could be recommended for those who type check their code), but it's food for thought, I guess.

[zanona]

After playing around a little with types, I could see I could get type-checking through csstype as:

import type CSS from 'csstype';

type Style = CSS.Properties | Record<string,CSS.Properties>;

const foo:Style = {
	colour: 'red', //TS Error: fail prop 'colour' doesn't exist
	':hover': { colour: 1} //TS Error: fail prop 'colour' doesn't exist and it's not a number
};

So this is correctly typed because Style will have properties defined as CSS properties or fallback to a free string when the content is an object. The problem, however, is that this cannot be sent to Emotion.css() as it expects a Interpolation object which is not compatible with the CSS.Properties extension type.

So far what I could see is that the CSSOthersObject can really accept anything, becoming as wide as a Record<string,any> so we're not getting too much value out of these typings. See below:

https://github.com/emotion-js/emotion/blob/fc119766b539f73546dc8e2863cca1439af3cb1c/packages/serialize/types/index.d.ts#L31-L40

where interpolation is: https://github.com/emotion-js/emotion/blob/fc119766b539f73546dc8e2863cca1439af3cb1c/packages/serialize/types/index.d.ts#L20-L29

I guess as we were previously discussing, all custom selectors will contain an object so we wouldn't need to allow for string as a result of ':hover': 'foo' but really expect ':hover':{...}.

[Andarist]

As to your Style example:

const styles = {
  color: 'hotpink',
  'div': {
    color: 'green'
  }
}

This styles object is not assignable to CSS.Properties, nor it is assignable to Record<string,CSS.Properties>. Why is it assignable to Style though? 🤔

Either way - I understand where are you coming from, but I'm very hesitant about changing this. The change would for example not allow to use css variables (without having to cast them or augmenting csstype interfaces). What we have has worked so far and I don't know if the increased type safety for this one justifies it being harder to use. Note that I'm not against changing what types we provide in general - upcoming v11 will come with an overhaul of the types and stricter Theme type-checking.