QuickBase / babel-plugin-styled-components-css-namespace

A babel plugin to add css namespaces (and increase specificity) of styled-component classes.
MIT License
118 stars 32 forks source link

Issue with nested pseudo-selectors in expressions #52

Open tinynumbers opened 5 years ago

tinynumbers commented 5 years ago

I posted https://github.com/QuickBase/babel-plugin-styled-components-css-namespace/pull/50 to handle the shuffling of expressions that happens when the CSS is run through postcss and the CSS contains nested pseudo-selectors e.g. &:hover, &:active etc.

That PR fixes one class of problems, but we've found another related problem that occurs when we include these sorts of pseudo-selectors inside the template expressions.

Example:

Here's the original styled component:

export default styled.div`
  background: ${props => props.background};
  border: 1px solid ${props => props.borderColor};
  cursor: text;

  &:hover {
    border-color: ${props => props.hoverBorder};
  }

  &:active {
    border-color: ${props => props.activeBorder};
  }

  ${props =>
    props.disabled &&
    css`
      border-color: ${props.disabledBorder};
      cursor: not-allowed;

      &:active,
      &:hover {
        border-color: ${props.disabledBorder};
      }

      * {
        cursor: not-allowed;
      }
    `
  };
`;

And here it is after processing with this plugin using cssNamespace: '.class-wrapper .other-wrapper':

export default styled.div`
.class-wrapper .other-wrapper & {
  background: ${props => props.background};
  border: 1px solid ${props => props.borderColor};
  cursor: text;

  ${props => 
    props.disabled &&
    css`
      border-color: ${props.disabledBorder};
      cursor: not-allowed;

      &:active,
      &:hover {
        border-color: ${props.disabledBorder};
      }

      * {
        cursor: not-allowed;
      }
    `
  };
}

.class-wrapper .other-wrapper &:hover {
  border-color: ${props => props.hoverBorder};
}

.class-wrapper .other-wrapper &:active {
  border-color: ${props => props.activeBorder};
}
`;

When styled-components renders this into the page and the component has the disabled prop set, the resulting CSS will look like this:

.class-wrapper .other-wrapper .AAAAAA {
  background: #bgcolor;
  border: 1px solid #borderColor;
  cursor: text;

  border-color: #disabledBorder;
  cursor: not-allowed;
}

.class-wrapper .other-wrapper .AAAAAA:active,
.class-wrapper .other-wrapper .AAAAAA:hover {
  border-color: #disabledBorder;
}

.class-wrapper .other-wrapper .AAAAAA * {
  cursor: not-allowed;
}

.class-wrapper .other-wrapper .AAAAAA:hover {
  border-color: #hoverBorder;
}

.class-wrapper .other-wrapper .AAAAAA:active {
  border-color: #activeBorder;
}

So, the problem is that this places the original (non-disabled) &:hover and &:active below the overrides that are intended to apply when the component gets the disabled prop. Therefore the "disabled" overrides don't actually override the base styles, but rather they themselves get overridden by the base styles, since CSS uses code-order to determine precedence.

The reason: this plugin "unwraps" the source styles, which pulls the &:hover and &:active that are not in expressions below the main style block. But the &:hover and &:active that are in expressions don't get "unwrapped" until styled-components generates the final CSS, based on evaluating those expressions. At that time, the expression versions get placed just below the main style block, before the "main" versions that were pulled out by the plugin.

Hopefully this makes sense.

My question:

It seems to me that the inclusion of the postcss-nested PostCSS processor is only to accommodate the postcss-parent-selector, which will generate invalid results if provided nested CSS. I wonder though if, instead of using postcss-parent-selector, instead a simplified version of that processor could be used, which only modifies the root element of the CSS it receives? Is there any reason for postcss-nested other than to make postcss-parent-selector happy? (*see below)

Such a simplified processor might look like this:

import postcss from 'postcss';

export default postcss.plugin('add-parent-selector', (opts = {}) => (root) => {
  root.walkRules((rule) => {
    if (rule.parent && rule.parent.type === 'root') {
      rule.selectors = rule.selectors.map((selectors) =>
        selectors.split(/,[\s]* /g).map((selector) => {
          // don't add the parent class to a rule that is
          // exactly equal to the one defined by the user
          if (selector === opts.selector) {
            return selector;
          }
          return `${opts.selector} ${selector}`;
        }),
      );
    }
  });
});

But I'm not sure if there are other reasons to include the postcss-nested processor that I am not seeing. Any thoughts?

I'd be happy to post this proposal as a PR but don't want to do so if it's going to break other things.


*Without unnesting, the parent-selector processor would produce something like...

export default styled.div`
.class-wrapper .other-wrapper & {
  background: ${props => props.background};
  ... 

  .class-wrapper .other-wrapper &:hover {
    border-color: ${props => props.hoverBorder};
  }

  .class-wrapper .other-wrapper &:active {
    border-color: ${props => props.activeBorder};
  }
  ...
}

`;

...i.e. generating final selectors like .class-wrapper .other-wrapper .xxxxxx .class-wrapper .other-wrapper .xxxxx:hover { ... }. Not good.

tinynumbers commented 5 years ago

I've spent further time experimenting with this and come to find that a number of valid-looking CSS blocks would fail if they are no un-nested before applying the parent selector; i.e. my proposed approach only works for a narrow set of cases.

Any other suggestions?