cssinjs / jss

JSS is an authoring tool for CSS which uses JavaScript as a host language.
https://cssinjs.org
MIT License
7.06k stars 397 forks source link

[jss-nested] Can't find referenced top-level rules inside of @ rules #848

Closed trusktr closed 5 years ago

trusktr commented 5 years ago

Expected behavior:

I was trying to reference $foo-style class name references media queries like the following and hoping it would work. (codesandbox reproduction)

Here is the raw object I'm using with react-jss:

{
  mapperLogo: {
    display: "block",
    position: "absolute",
    top: "50%",
    left: 0,
    transform: "translate(0,-50%)",
    "& img": {
      display: "block"
    },
    "@media (max-width: 69.686875em)": {
      paddingTop: "0.4375rem",
      marginLeft: "1.61875rem",
      "& img": {
        width: "12.5rem"
      }
    },
    "@media (min-width: 69.6875em)": {
      marginLeft: "2.3125rem",
      marginTop: "0.1875rem",
      zIndex: "1000",
      "& img": {
        height: "1.6875rem"
      }
    }
  },
  header: {
    position: "fixed",
    top: 0,
    left: 0,
    right: 0,
    width: "100%",
    backgroundColor: "#f1f1f1",
    zIndex: 100,
    "@media (max-width: 69.686875em)": {
      height: "6.25rem",
      "& $menuItem": {
        marginLeft: "1.25rem",
        marginRight: "1.25rem",
        float: "right"
      },
      "& $menu": {
        margin: 0,
        position: "absolute",
        top: "50%",
        right: 0,
        transform: "translate(0%,-50%)",
        "& $menuItem": {
          display: "none"
        },
        "& $ham": {
          display: "block",
          padding: [["0.875rem", "1.61875rem"]],
          "& img": {
            width: "3.5rem"
          }
        }
      }
    },
    "@media (min-width: 69.6875em)": {
      height: "3.125rem",
      textAlign: "left",
      "& $menu": {
        float: "right",
        margin: 0,
        position: "absolute",
        top: "50%",
        right: "1.25rem",
        transform: "translate(0%,-50%)",
        display: "flex",
        fontSize: "1.25rem",
        "& $menuItem": {
          marginLeft: "2.5rem",
          marginRight: "2.5rem",
          display: "block",
          color: "#898989",
          fontWeight: "bold",
          textDecoration: "none",
          fontFamily: "SF, sans-serif",
          textAlign: "center",
          letterSpacing: "0.0625rem",
          "&:hover, &:active, &:focus, &$currentPage": {
            fontWeight: "bold",
            fontFamily: "SFBold",
            letterSpacing: 0,
            color: "black"
          }
        },
        "& $ham": {
          display: "none"
        }
      }
    },
    opacity: 1,
    transition: "opacity 200ms",
    "@supports ( -webkit-backdrop-filter: blur(40px) ) or ( backdrop-filter: blur(40px) )": {
      opacity: 0.8
    },
    backdropFilter: "blur( 2.5rem )",
    "&.faded": {
      opacity: 0.8,
      transition: "opacity 200ms"
    }
  },
  menu: {},
  menuItem: {},
  currentPage: {},
  ham: {}
}

Describe the bug:

I get some errors like these:

screen shot 2018-09-10 at 3 29 48 pm

Codesandbox link:

None yet.

Versions (please complete the following information):


Any ideas? Do you see anything obviously wrong? Are top-level @ rules with references supported?

trusktr commented 5 years ago

The above object is generated from a function which I use with react-jss,

import withStyle from 'react-jss'

function style() {
  return // the above object
}

withStyle(style)(MyComponent)
trusktr commented 5 years ago

Hmmm, also if you look at the bottom of the following screenshot, you'll see something strange like

.Header-header-0-1-4 menu menuItemcurrentPage

which is supposed to look like

.Header-header-0-1-4 .menu-0-1-5 .menuItem-0-1-6.currentPage-0-1-7

screen shot 2018-09-10 at 4 22 37 pm

HenriBeck commented 5 years ago

Please create a minimal codesandbox example with the error.

trusktr commented 5 years ago

@HenriBeck Here's a codesandbox reproduction. See the errors in the console: https://codesandbox.io/s/wyplxrr7lw

HenriBeck commented 5 years ago

Yeah, seems to be a bug with nested rules inside nested @ rules.

I think it's not passing the correct container stylesheet.

trusktr commented 5 years ago

Even if the @ rule is not nested, it has the same problem. Here's an example with @media at the top level of the stylesheet: https://codesandbox.io/s/3r8p836m95

HenriBeck commented 5 years ago

Nope, see how media queries work on the top level: http://cssinjs.org/json-api?v=v9.8.7#media-queries

Example: https://codesandbox.io/s/qx87v60ymj

trusktr commented 5 years ago

Yeah, but you don't have a non-media-query instance of header or mapperLogo at the top level. How do we do that?

Another try, this time the error shows the interpolated class names: https://codesandbox.io/s/zrk5yl3793

trusktr commented 5 years ago

oh okay, I see, just header or mapperLogo inside the top-level at-rule, even if those are also at the top level.

trusktr commented 5 years ago

Alright, got it is working with top-level queries: https://codesandbox.io/s/611m4rn25w

I'll use this workaround for now (unless there's a way to make nested queries work?).

trusktr commented 5 years ago

@HenriBeck Notice that it requires the empty objects inside the media queries too, otherwise it'll give the same type of errors if you remove for example the ham: {} from inside the media queries.

So in this sense, top-level queries are affected too. Compare these two examples,

and notice the only difference is on line 45, where I remove ham: {}.

Basically, if ham: {} is already at the top level, it isn't intuitive to think to also add it inside the media query.

HenriBeck commented 5 years ago

Yeah, will need to see what works and what's broken. Don't know when we will have time to fix this though. If you can, avoid the jss-nested syntax with multiple nested rules for now.

kof commented 5 years ago

I assume it looks only one level up when it looks for a ref.

trusktr commented 5 years ago

I assume it looks only one level up when it looks for a ref.

Not necessarily (or maybe your concept of "one level up" is different?).

Compare the last two examples, on line 45, with and without ham: {}: if the at-rule does not also contain the ham: {} object, then it won't find it from the "top level" object. So it has an error when we remove ham: {} from the at-rule.

kof commented 5 years ago

@trusktr for e.g. you could create a PR with a test that reproduces the issue.

trusktr commented 5 years ago

The above code sandbox shows the error in the console.

HenriBeck commented 5 years ago

The test would help us find the issue more quickly

trusktr commented 5 years ago

Alright, when if I get a chance. I'm in crunch mode for a project with first release in 6 days. Next time I report a bug, I'll just report it as a test.

sammi commented 5 years ago

Here is the test code to reproduce the issue.

 describe('nest rules inside media query', () => {
    let sheet

    beforeEach(() => {
      const localJss = create(settings).use(nested())
      sheet = localJss.createStyleSheet({
        card: {},
        cardDeck: {
          '@media (min-width: 576px)': {
            '& $card': {
              margin: '15px'
            }
          }
        }
      })
    })

    it('should generate nested dynamic card styles', () => {
      expect(sheet.toString()).to.be(stripIndent`
        @media (min-width: 576px) {
          .cardDeck-id card-id {
            margin: 15px;
          }
        }
      `)
    })
  })

Current jss-nested plugin returns:

@media (min-width: 576px) {
  .cardDeck-id card {
    margin: 15px;
  }
}