WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.4k stars 4.16k forks source link

Add ESLint rule to prevent usage of the order CSS property #61247

Open afercia opened 5 months ago

afercia commented 5 months ago

Description

See https://github.com/WordPress/gutenberg/issues/61241

Some components come with CSS in JS.

https://github.com/WordPress/gutenberg/issues/61241 and https://github.com/WordPress/gutenberg/pull/61243 aim to introduce a stylelint rule to prevent usage of the order CSS property, which is impactful for accessibility.

In the same way, it would be nice to add an ESLint rule to prevent usage of the order CSS property in any *.js file.

Unfortunately this isn't exactly my area of expertise so that I'd appreciate some guidance. I did see a similar rule added in https://github.com/WordPress/gutenberg/pull/58130

Step-by-step reproduction instructions

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

afercia commented 5 months ago

I guess the patterns to catch would be, at least, the following ones. With and without leading spaces or tab characters:

order: 123;
    order: 123;

order: 123,
    order: 123,

order: '123',
    order: '123',

'order: 123;'
    'order: 123;'
ghostp13409 commented 3 months ago

Hey, I'm fairly new to open-source and web development, but I would love to give it a try

afercia commented 3 months ago

@ghostp13409 welcome. Sure, please do feel free to work on a Pull Request.

I did try to follow a previous example that added another ESLint rule, see https://github.com/WordPress/gutenberg/pull/59022. But I'm not good at regular expressions. Also, I have no idea how that works. I did try to go through the ESLint custom rules docs and the docs for selectors but this isn't really my area of expertise. Wondering if there's any simpler way to detect the order CSS property in JS. @mirka any help would be appreciated, when you have a chance 🙇🏽‍♂️

It gets complicated because of the several ways CSS can be coded in JS. It could be a key/value pair within an object, or a string, or a concatenated string... etc.

Note the JS linter can be run in the terminal with this command: npm run lint:js -- --quiet

mirka commented 3 months ago

CSS-in-JS can either be done with strings or objects. The string case can be caught similar to #59022 without much downside, but the object case will be trickier because { order: 3 } is something that could validly appear in code unrelated to styling.

I wouldn't say this rule is particularly necessary at the moment, given that:

Might be fine to revisit once we actually encounter an order slipping through.