hudochenkov / postcss-styled-syntax

PostCSS syntax for CSS-in-JS like styled-components
MIT License
69 stars 4 forks source link

Expected single space before "==" (scss/operator-no-unspaced) Stylelint scss/operator-no-unspaced #9

Closed karlhorky closed 1 year ago

karlhorky commented 1 year ago

Hi @hudochenkov , first of all thanks again for this syntax, really great! 🙌

When using === in an expression in an interpolation in a css tagged template literal, the scss/operator-no-unspaced rule complains that there is no space before or after the ===, although it should recognize that this is an interpolation and not a part of the SCSS:

import { css } from '@emotion/react';

const variant = { size: 'small' };
const aStyles = css`
  > div {
    padding-left: ${variant.size === 'small' ? '35px' : '64px'}; /* 💥 Expected single space before "==" (scss/operator-no-unspaced) Stylelint scss/operator-no-unspaced */
  }
`;
Screenshot 2023-02-17 at 17 37 32
$ npx stylelint 'packages/**/*.{js,tsx}'

packages/cargobay/components/Button.tsx
  99:39  ✖  Expected single space after "=="   scss/operator-no-unspaced
  99:39  ✖  Expected single space before "=="  scss/operator-no-unspaced
 100:40  ✖  Expected single space after "=="   scss/operator-no-unspaced
 100:40  ✖  Expected single space before "=="  scss/operator-no-unspaced

Configuration

stylelint.config.cjs

/** @type { import('stylelint').Config } */
const config = {
  extends: [
    'stylelint-config-recommended',
    'stylelint-config-recommended-scss',
    'stylelint-config-css-modules',
  ],
  rules: {
    'no-descending-specificity': null,
    // Allow files without any styles
    'no-empty-source': null,
  },
  overrides: [
    {
      files: [
        '**/*.js',
        '**/*.cjs',
        '**/*.mjs',
        '**/*.jsx',
        '**/*.ts',
        '**/*.tsx',
      ],
      customSyntax: 'postcss-styled-syntax',
    },
  ],
};

module.exports = config;

package.json

{
  "devDependencies": {
    "postcss": "8.4.21",
    "postcss-styled-syntax": "0.3.1",
    "stylelint": "15.1.0",
    "stylelint-config-css-modules": "4.2.0",
    "stylelint-config-recommended": "10.0.1",
    "stylelint-config-recommended-scss": "9.0.1"
  }
}

Also applies to nested css tagged template literals

Starting with postcss-styled-syntax@0.3.2, this nested syntax also fails (I guess the inner css tagged template literal was not being checked before this version):

const variant = { size: 'small' };
const aStyles = css`
  ${css`
    > div {
      padding-left: ${variant.size === 'small' ? '35px' : '64px'};
    }
  `}
`;
hudochenkov commented 1 year ago

I think the best solution is to disable rule, which doesn't apply to CSS-in-JS syntax.

karlhorky commented 1 year ago

Thanks for the reply! I guess that's a temporary workaround, to temporarily disable the rule.

But I think it would be good if postcss-styled-syntax parsed this correctly, eg. resolved to the following SCSS options:

> div {
  padding-left: 35px;
}

and

> div {
  padding-left: 64px;
}

(which could then have all linting rules applied to them)

karlhorky commented 1 year ago

Or if it's complex to resolve all possible versions of the static SCSS, maybe a recommended alternative pattern like the code below that avoided the problem without an ignore / disable would be ok... 🤔 :

const variant = { size: 'small' };
const aStyles = css`
  > div {
    ${variant.size === 'small' ?
      css`padding-left: 35px;` :
      css`padding-left: 64px;`
    }
  }
`;
hudochenkov commented 1 year ago

scss/operator-no-unspaced is not built to work with CSS-in-JS. When this rule see declaration value as ${variant.size === 'small' ? '35px' : '64px'} it doesn't know what to do with such value. PostCSS AST is not built for CSS-in-JS either. We have to fit into what is available in PostCSS. So the only thing we can do is to have declaration value as a string shown above.

postcss-styled-syntax is a parser. It would not try to evaluate JavaScript in attempt to get value that might be there. It is too complicated, and it might need to build something like TypeScript to do that.

What is the use for this rule for CSS-in-JS?

P. S. In any case we're talking about some plugin rule, and it not related to this syntax.

karlhorky commented 1 year ago

Ok, so maybe it's our Stylelint configuration that should be adapted (eg. maybe I should have 2-3 overrides blocks instead of just one, so that the stylelint-config-recommended-scss doesn't apply to JS/TS files):

/** @type { import('stylelint').Config } */
const config = {
  extends: [
    'stylelint-config-recommended',
    'stylelint-config-recommended-scss',
    'stylelint-config-css-modules',
  ],
  rules: {
    'no-descending-specificity': null,
    // Allow files without any styles
    'no-empty-source': null,
  },
  overrides: [
    {
      files: [
        '**/*.js',
        '**/*.cjs',
        '**/*.mjs',
        '**/*.jsx',
        '**/*.ts',
        '**/*.tsx',
      ],
      customSyntax: 'postcss-styled-syntax',
    },
  ],
};

module.exports = config;
hudochenkov commented 1 year ago

Sounds like a good solution.

karlhorky commented 1 year ago

This is the config I ended up with, closing now:

/** @type { import('stylelint').Config } */
const config = {
  extends: ['stylelint-config-recommended'],
  rules: {
    'no-descending-specificity': null,
    // Allow files without any styles
    'no-empty-source': null,
  },
  overrides: [
    {
      files: ['**/*.css', '**/*.scss', '**/*.sass', '**/*.less'],
      extends: ['stylelint-config-recommended-scss'],
    },
    {
      files: ['**/*.module.css', '**/*.module.scss', '**/*.module.sass'],
      extends: ['stylelint-config-css-modules'],
    },
    {
      files: [
        '**/*.js',
        '**/*.cjs',
        '**/*.mjs',
        '**/*.jsx',
        '**/*.ts',
        '**/*.tsx',
      ],
      customSyntax: 'postcss-styled-syntax',
    },
  ],
};

module.exports = config;