facebook / stylex

StyleX is the styling system for ambitious user interfaces.
https://stylexjs.com
MIT License
8.21k stars 304 forks source link

Nested pseudo classes within pseudo elements #550

Open larsfoll opened 2 months ago

larsfoll commented 2 months ago

Describe the issue

According to the @stylexjs/eslint-plugin valid styles rules, I cannot nest more than 1 level deep. Inside '::webkite-scrollbar-thumb', however the styles are applied and everything works.

Is this expected behavior? I would assume the code either fails to compile or the styles are not applied. Or is it possible to do this and is the eslint rule more like a suggestion to not do this.

image

Expected behavior

Either the nested styles are not applied or the code fails to compile.

Steps to reproduce

Install necessary dependencies and add the linter to the eslint config

    "@stylexjs/eslint-plugin": "0.6.0",
    "@stylexjs/stylex": "0.6.0",

Write nested styles inside ::-webkit-scrollbar-thumb

import * as stylex from '@stylexjs/stylex';

const styles = stylex.create({
  content: {
    gridArea: 'content',
    overflowX: 'hidden',
    overflowY: 'auto',
    '::-webkit-scrollbar-thumb': {
      borderRadius: '6px',
      backgroundColor: { default: 'rgba(0, 0, 0, 0.2)', ':hover': 'rgba(0, 0, 0, 0.4)' },
    },
  },
});

Test case

No response

Additional comments

No response

nmn commented 2 months ago

This is a valid issue. You can suppress the ESLint error for now.

necolas commented 2 months ago

Surely the better workaround is to ignore the eslint-plugin's warning? Isn't the correct syntax the one in the OP, not the one in the workaround above? I think pseudo-elements shouldn't be allowed within property object-values - that doesn't really make sense to me.

The role of the eslint-plugin should be limited to stylistic things like "sort keys", or other opinionated preferences like "don't use px values for font-size", not for validating syntax or allowed properties.

nmn commented 2 months ago

@necolas My bad. I assumed it was a bug in the Babel plugin because it used to be a bug at one point. You're right, the best solution here is to just suppress the ESLint error.

nedjulius commented 2 months ago

should this be removed from the plugin?

https://github.com/facebook/stylex/blob/bea3495859da52fa1d581df409b1dac8f7e4e934/packages/eslint-plugin/src/stylex-valid-styles.js#L2481

there is a TODO comment that reminds to remove this suggestion as well

nmn commented 2 months ago

should this be removed from the plugin?

With some extra checks to disallow it for anything but pseudo elements.