stylex icon indicating copy to clipboard operation
stylex copied to clipboard

@stylexjs/eslint-plugin: Better parsing to correctly validate various values.

Open robinlahtinen opened this issue 1 year ago • 6 comments

Describe the issue

Stylex ESLint disallows oklch(0 0 0) or following color formats.

Expected behavior

Stylex ESLint allows all color formats supported by CSS Color Module Level 4 or higher.

Steps to reproduce

  1. Create a style with color property
  2. Use value outside the type, for example oklch(0 0 0).
  3. Try linter

Test case

const styles = stylex.create({
    example: {
        color: "oklch(0 0 0)",
    },
});

Additional comments

No response

robinlahtinen avatar Feb 04 '24 19:02 robinlahtinen

The ESLint rule seems to already support any string as we have: const color = makeUnionRule(isString, isNamedColor, isHexColor). Plus, I'm not getting any error with the given example with the latest version of the plugin, which is 0.5.1.

yousoumar avatar Feb 05 '24 06:02 yousoumar

The ESLint rule seems to already support any string as we have: const color = makeUnionRule(isString, isNamedColor, isHexColor). Plus, I'm not getting any error with the given example with the latest version of the plugin, which is 0.5.1.

I think the OP wants to have linting support for color formats such as oklch, rgba etc. based on the current implementation, eslint just treats these formats as strings and does not report any problem with invalid values.

nedjulius avatar Feb 05 '24 16:02 nedjulius

Sorry, in a hurry I misunderstood I guess. Thanks @nedjulius!

yousoumar avatar Feb 05 '24 17:02 yousoumar

A bigger improvement to the ESLint rule is planned. The plan is to use a CSS value parser to validate all CSS properties according to the specification and remove the need to accept arbitrary strings when specific patterns are expected.

I have worked on a custom CSS parser for this use-case since postcss-value-parser didn't do what we needed. The parser can be found here.

We will get back to working on this once the most urgent issues have been resolved.

nmn avatar Feb 07 '24 04:02 nmn

So should we wait for the parser before working on this issue @nmn? As a matter of fact, I started looking at it, implemented already a validation for rgb, rgba, hsl and hsla. They are working fine with normal strings, but not with template literals. Firstly I thought we should only have normal strings as the doc says, but I found one template literal in the Next.js example app.

yousoumar avatar Feb 07 '24 08:02 yousoumar

@yousoumar Please wait for the parser to start working on this. There is a bigger refactor needed for the ESLint plugin to consistent parse complex values that are currently parsed in the Babel plugin.

Once this refactor is done, the validation logic will be implemented in vanilla JS with no AST dependency. This is similar to how the shared package contains all the core logic for the StyleX Babel plugin and operates on vanilla strings, objects and arrays.

nmn avatar Feb 08 '24 00:02 nmn