Skip to content
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

Typescript Type support for style objects #1267

Closed
carpben opened this issue Mar 12, 2019 · 6 comments
Closed

Typescript Type support for style objects #1267

carpben opened this issue Mar 12, 2019 · 6 comments

Comments

@carpben
Copy link

carpben commented Mar 12, 2019

I'm working with React Typescript and Emotion. So far I have used template strings, but I'd like to gradually move to style objects.
I'd like to be able to declare a style object, get type support from the editor, and then pass it to a css function, or a styled component.
CSSProperties type by @types/react provides such support (only valid css properties could be keys). but emotion css function doesn't accept parameters of that type. It does accept CSSObject (by Emotion), but that type accepts any key.
The solution I currently use is:

export type CSSObj = Pick<CSSObject, keyof CSSProperties>

This works well. Is there a "native" emotion way to do this? If there isn't, perhaps such a type could be added. I'd be happy to prepare a PR.

  • emotion version: 9.2.12
  • react version: 16.7
@JoshRosenstein
Copy link

The reason for this is to allow nestability and usability with css selectors.
CSSObject extends CSSProperties which is the same source where @types/react gets CSSProperties (csstype )

import * as CSS from 'csstype'
import { Equal } from './helper'
export { RegisteredCache, SerializedStyles }
export type CSSProperties = CSS.PropertiesFallback<number | string>

export interface CSSObject
extends CSSPropertiesWithMultiValues,
CSSPseudosForCSSObject,
CSSOthersObjectForCSSObject {}

So

export type CSSObj = Pick<CSSObject, keyof CSSProperties> 
=== CSSPropertiesWithMultiValues
===   {[K in keyof CSSProperties]:
               | CSSProperties[K]
               | Array<Extract<CSSProperties[K], string>>}

Therefore if your not passing any Arrays to your css properties (Array<Extract<CSSProperties[K], string>>), and dont have any nested styles, Then you can just simply use CSSProperties from @react/types or @emotion/serialize or csstype

@thisissami
Copy link

thisissami commented Jun 15, 2019

Hey just following up on this: I think it would be really useful if @emotion/core exported CSSPropertiesWithMultiValues.

If I have code as follows, TypeScript complains:

const cssObj: CSSProperties = {
  display: 'flex',
  justifyContent: 'space-between',
  alignItems: 'baseline'
};

const App = () => <div css={cssObj}/>;

This (I believe) is contrary to what you expressed above @JoshRosenstein . On the other hand, if I import CSSPropertiesWithMultiValues, then TypeScript stops complaining.

Given that in a react + typescript + emotion project, it so ubiquitous to need to define types as above - I think it would be great if @emotion/core exported a Type of this sort so that we don't need to import from both @emotion/core and @emotion/serialize in every file that we use emotion. (for a codebase where CSSObjects are defined as their own variables, anyway).

My $0.02 would also be that it would be nice to have a shorter alias for such an export... maybe CSSObjectForReact, ReactCSSObject, or CSSObjectForJSX?

What are your thoughts?

(this is all in emotion v10.10 & react v16.8 btw)

@thisissami
Copy link

Just one update: this is the type that I use, if anybody is curious.

import { CSSPropertiesWithMultiValues } from '@emotion/serialize';

export type CSSObj = CSSPropertiesWithMultiValues & { label?: string };

@Andarist
Copy link
Member

@JakeGinnivan could you take a look at this?

@JakeGinnivan
Copy link
Contributor

I think the types in the serialize package need a go over. Personally I don't use the babel plugin so we do this:

const cssObj: CSSProperties = {
  display: 'flex',
  justifyContent: 'space-between',
  alignItems: 'baseline'
};

const StyledDiv = styled.div(cssObj)

And create a dedicated component with those styles so I haven't hit this myself.

If you do want to have a go at preparing a PR, it will not conflict with my PR. That said my PR gets all the tests passing so you know your changes haven't broken anything. So maybe base any PR off #1501.

@Andarist
Copy link
Member

So to sum it up - if this is still something that is problematic, please raise a PR against #1501. Other than that I don't think there is anything else actionable here right now, so I'm going to close the issue to clean up the issue tracker.

Be sure though that we care about DX a lot so if this still somehow bothers you and you cannot raise a PR yourself then please open a new issue explaining the exact problem and ideally with proposed solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants