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] createStyles is incompatible with top level @media queries #12277

Closed
2 tasks done
dsenkus opened this issue Jul 25, 2018 · 5 comments
Closed
2 tasks done
Labels
out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future) typescript

Comments

@dsenkus
Copy link

dsenkus commented Jul 25, 2018

Typescript compiler fails when using @media queries as top level keys in styles.

  • This is a v1.x issue.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

Ideally @media queries should be allowed as top level keys in styles.

Current Behavior

Typescript error:

@media (min-width:...' is not assignable to parameter of type 'Record<string, CSSProperties>'

Full error message:

(4,47): Argument of type '{ root: { minHeight: string; }; content: { minHeight: string; }; sidebar: {}; '@media (min-width:...' is not assignable to parameter of type 'Record<string, CSSProperties>'.
  Property ''@media (min-width: 960px)'' is incompatible with index signature.
    Type '{ root: { display: string; }; content: { flexGrow: number; }; sidebar: { width: string; }; }' is not assignable to type 'CSSProperties'.
      Types of property 'content' are incompatible.
        Type '{ flexGrow: number; }' is not assignable to type 'string | undefined'.
          Type '{ flexGrow: number; }' is not assignable to type 'string'.
g

Steps to Reproduce

import * as React from 'react';
import { createStyles, WithStyles, withStyles, Theme } from '@material-ui/core';

const styles = (theme: Theme) => createStyles({
  root: {
    minHeight: '100vh',
  },
  content: {
    minHeight: '100vh',
  },
  sidebar: {
  },
  '@media (min-width: 960px)': {
    root: {
      display: 'flex',
    },
    content: {
      flexGrow: 1,
    },
    sidebar: {
      width: '250px',
    },
  }
});

interface Props extends WithStyles<typeof styles> {
}

const TestComponent: React.SFC<Props> = () => (<p>It Works!</p>);

export default withStyles(styles)(TestComponent);

Context

Although having @media queries embedded within classes works, but it would be great to have them as top level style elements:

// this works
const styles = (theme: Theme) => createStyles({
  root: {
    minHeight: '100vh',
    '@media (min-width: 960px)': {
      display: 'flex',
    }
  }
})

// however this would be really useful:
const styles = (theme: Theme) => createStyles({
  root: {
    minHeight: '100vh'
  },
  '@media (min-width: 960px)': {
    root: {
      display: 'flex',
    },
  }
})

Your Environment

Tech Version
Material-UI v1.4.1
React v16.41
tsc v2.9.2
@dsenkus dsenkus changed the title [typescript] createStyles is incopatible with @media queries [typescript] createStyles is incompatible with top level @media queries Jul 25, 2018
@eps1lon
Copy link
Member

eps1lon commented Aug 30, 2018

createStyles allows top level media queries as of #11007. The problem here is that typescript thinks content is the css property content and not a className. Resolving this via definitions might be impossible. Your best bet is to use only classnames that don't conflict with existing CSS properties.

I'm going to add a test to avoid regression and illustrate the issue. Might be good to add a note to the documentation.

@rosskevin
Copy link
Member

rosskevin commented Sep 5, 2018

tsc wasn't enabled in the ci build, this exact issue was just proven with #12783

@oliviertassinari
Copy link
Member

It seems to be working now with TypeScript v3.1.1.

@merceyz
Copy link
Member

merceyz commented Jun 5, 2019

The problem here is that typescript thinks content is the css property content and not a className. Resolving this via definitions might be impossible

@eps1lon It would be possible to let all CSS properties be set to a CSSProperties object, though that might not be something that's wanted.
Could be added here: https://github.com/mui-org/material-ui/pull/15546/files#diff-2420cff07893e7d10e918d0c2fffdc9bR14

Otherwise everything is working as expected and this issue can be closed

@eps1lon
Copy link
Member

eps1lon commented Aug 28, 2019

Closing as working as intended. See #12277 (comment) for explanation and workaround.

@eps1lon eps1lon closed this as completed Aug 28, 2019
@oliviertassinari oliviertassinari added the out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future) label Jan 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
out of scope The problem looks valid but we won't fix it (maybe we will revisit it in the future) typescript
Projects
None yet
Development

No branches or pull requests

5 participants