[jss-nested] Support deep nesting

[cvle]

When I updated JSS i suddenly saw a lot of warnings popping up in my console and saw that deep nesting support was disabled with https://github.com/cssinjs/jss-nested/pull/11.

I would love to see deep level nesting supported again for my use case:

I'm using a 3 level approach for styling my elements: element -> variant -> state. One example would be button -> flat -> focused.

In Code:

  button: {
    ...
    "&.flat:": {
      ...
      "&:focus": {
        ...
      },
    },
    "&.raised:": {
      ...
      "&:focus": {
        ...
      },
    },
  }

Previously I was only using 2 levels element+variant -> state but this mixes up elements and variants and it felt wrong to me. In a component with many variants it got very confusing and it was hard to tell the different stylable parts of the component. After changing to the 3-level-approach my components code got a lot clearer. The styling code suffers indeed a bit from readability but it is more logical and for me this is more worth than the extra readability.

Even though JSS started to complain about my nested styles, it still seems to work, so I guess it still works as before, but you don't plan to support it anymore?

What do you think @kof?

[kof]

Can you give me a real world jss example? The version you think doesn't look good, with 2 level nesting.

[cvle]

This is a simplified version of my button style:

 {
   root: {
     /* Basic Styling. */
     borderRadius: "2px",
     height: "2.25rem",
     lineHeight: "2.25rem",
     padding: "0rem 0.5rem",
     transition: "all 550ms cubic-bezier(0.23, 1, 0.32, 1) 0ms",
     minWidth: "4rem",
     backgroundColor: "transparent",
     color: "black",
     "&:hover": {
       background: "rgba(0, 0, 0, 0.12)",
     },

     /* Dark variant */
     "&.dark": {
       color: "white",
       // State.
       "&:hover": {
         background:"rgba(255, 255, 255, 0.12)",
       },
     },

     /* Raised variant */
     "&.raised": {
       padding: "0rem 1rem",
       boxShadow: shadow,
       color: "black",
       background: "grey",
       // State.
       "&:hover": {
         background: "darkergrey",
       },
       "&:focus": {
         background: "anothercolor",
       },
     },

     /* Floating variant */
     "&.floating": {
       padding: "0rem 1rem",
       boxShadow: shadow,
       color: "black",
       background: "grey",
       borderRadius: "50%",
       padding: 0,
       minWidth: 0,
       width: "3.5rem",
       height: "3.5rem",
       lineHeight: "3.5rem",
       boxShadow: shadow,
       overflow: "hidden",
       // State.
       "&:hover": {
         background: "darkergrey",
       },
     },
   },
   label: {
     // label styles.
     // might as well style differently depending on which variants are active.
   },
   icon: {
     // icon styles.
   },
 }

Conceptually I apply variants to the whole component and add the variant classes to each stylable element. This allows the styles of every customizable part to be fully aware of the current variants, giving full flexibility for customization to the component user.

Additionally I allow certain variants to be mixed. The full version of my button has for example the variants "primary" and "accent", which when applied changes the colors of the button. For example a common button on my project has the following classes: mycomponentbutton-root raised accent with-icon-before.

Because of this approach some of my components need to use a tree with 3 levels.

[kof]

So, would it be that bad if you use this, it is a little duplication, but is that so bad and worth deep nesting?

{
  root: {
  /* Basic Styling. */
  borderRadius: "2px",
  height: "2.25rem",
  lineHeight: "2.25rem",
  padding: "0rem 0.5rem",
  transition: "all 550ms cubic-bezier(0.23, 1, 0.32, 1) 0ms",
  minWidth: "4rem",
  backgroundColor: "transparent",
  color: "black",
  "&:hover": {
    background: "rgba(0, 0, 0, 0.12)",
  },
  /* Dark variant */
  "&.dark": {
    color: "white",
  },
  // State.
  "&.dark:hover": {
    background:"rgba(255, 255, 255, 0.12)",
  }
}

[cvle]

It is not a big deal in a small example like this, but saves me around 30 times in my more advanced button component to write things like this

// My variants are statically defined, as well as some custom states.

[`&.${Variant.dark}:focus`]: {
  background: "rgba(255, 255, 255, 0.12)",
}
[`&.${Variant.dark}:hover`]: {
  background: "rgba(255, 255, 255, 0.12)",
}

instead of:

[`&.${Variant.dark}`]: {
  "&:hover": {
    background: "rgba(255, 255, 255, 0.12)",
  },
  "&:focus": {
    background: "rgba(255, 255, 255, 0.12)",
  }
}

Another thing is that I sometimes extend from variants:

const raised = {
   "&:focus": {
     background: "whatever",
   },
   "&:hover": {
     background: "darkergrey",
   },
   // more styling
};

const styles = {
   root: {
     // Basic Styling.

     "&.raised": raised,

     "&.floating": {
        extend: raised,
        // more styling
     },
}

I wouldn't be able to do this without the ES7 spread operator which is not supported in typescript yet until the 2.1 release. ( Wish I could extend from nested styles, but that is another topic ).

So for me this feature helps a lot in structuring my styles. The indentation helps me group related parts together and keep my overview. Without this feature I would have no obvious way to extend my variants as the ES7 spread operator support in typescript yet, though this will change in the future.

If the implementation of this feature is not too complex, I would argue that this is a useful and beneficial feature, especially for users coming from traditional css background, who are used to do nesting with SASS, LESS, postcss and co.

What are the arguments against enabling this feature?

[vass-david]

I'm in this with @cvle. This happend to me in exact same situation (buttons). Is there any problem with implemented deep nesting other that it's not a good habit? Don't get me wrong. I don't say opposite, I think it's wrong as well, but 3 levels is not that deep IMHO and enhances readability and removes unnecessary repetition a lot in this particular case (and more other similar cases).

[kof]

Well, I was waiting for a real life use case which you seem to have delivered ( @nathanmarks wdyt? ). Now we can think of increasing the default nesting level to 3 levels and create an option maxLevel. Also we would need to split the parent selector by comma and use every part as a value for childs & replacement.

[kof]

I think this has not the highest prio right now, because I think things on the roadmap are more important https://github.com/cssinjs/jss/#roadmap. It doesn't mean if someone sends me a good PR, we wont take it), its just I need to focus my resources.

[cvle]

Unfortunately I have no resources at the moment, so I appreciate anyone taking over this.

[nathanmarks]

@kof One option is to have the system work with N levels, and leave performance implications up to the user. As long as the implementation doesn't affect performance of nesting 1-2 levels I think this would be a good tradeoff.

If the user decides to nest 6 levels in every sheet -- the performance implications are their responsibility.

Having the nesting work with N levels would also most likely mean that the nesting implementation is a little more robust and less prone to having issues when other plugins are added.

[kof]

So here is the todo

• remove nesting level warning, leave it up to the user and linter (in the future)

• Make json like this work. For this, we need o split '& .a, & .b' by comma and make them separate rules

  {
    button: {
      '& .a, & .b': {
        '& .c': {
           color: 'red'
         }
       }
    }
  }
  
  => 
  
  {
    button: {
      '& .a': {
        '& .c': {
           color: 'red'
         }
       },
      '& .b': {
        '& .c': {
           color: 'red'
         }
       }
    }
  }
  
  => 
  
  .button-12345 .a .c,
  .button-12345 .b .c {
    color: red;
  }
  

[kof]

is someone on it?

[cvle]

I can give it a go this weekend

[cvle]

What should we do with the rule '& .a, .b'?

1. Treat it the same as '& .a, & .b' or even substitute it as the only valid syntax
2. Consider invalid, warn and skip processing
3. Produce one nested rule with .a and one at the root level with .b

The most simple solution would be no 1 where we just consider everything after & nested.

[kof]

Just checked with sass, they go also for 1 - scoping all comma separated selectors.

[kof]

Released with [email protected]

[AlvesJorge]

Well, I was waiting for a real life use case which you seem to have delivered ( @nathanmarks wdyt? ). Now we can think of increasing the default nesting level to 3 levels and create an option maxLevel. Also we would need to split the parent selector by comma and use every part as a value for childs & replacement.

Just stumbled upon this thread , and wow what an amazing attitude.
Thank you for maintaining such a great tool , kudos to you sir
Danke