Skip to content

Conversation

C-Lodder
Copy link
Member

@C-Lodder C-Lodder commented Sep 19, 2025

Summary of Changes

This migrates linting from ESLint to Biome, for 2 simple reasons:

  1. Performance - Biome is at least 15x faster than ESLint due to the fact it's written in Rust and it multi-threadded.
  2. Less dependencies - ESLint requires multiple plugins to work with languages and frameworks/libraries, whereas Biome supports them out of the box.

As part of this PR, I've also run the built-in formatter to try and use concise coding standards across the board.

To-Do

  • Fix some remaining CSS lint issues
  • Migrate CSS ordering rules
  • Remove lint:css task from package.json
  • Update Github action

@Fedik @dgrammatiko Would be nice to get your thoughts

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-6.0-dev labels Sep 19, 2025
@laoneo
Copy link
Member

laoneo commented Sep 19, 2025

Did you add a line length? Would probably make sense to increase it, to have less changes in this pr.

@C-Lodder
Copy link
Member Author

@laoneo The default is 80, which I believe is the same as ESLint.

Happy to increase this if you'd like a specific value, although 80 is generally considered a good number.

@laoneo
Copy link
Member

laoneo commented Sep 19, 2025

If I remember correctly we were going in the past with 150. But can't find any evidence right now. Perhaps @wilsonge can shed some light here. Personally, I find 80 to close.

@laoneo
Copy link
Member

laoneo commented Sep 19, 2025

These here are definitely incorrect.

@@ -19,8 +19,7 @@
"build:com_media:dev": "node --env-file=./build/development.env build/build.mjs --com-media",
"watch": "node build/build.mjs --watch",
"watch:com_media": "node build/build.mjs --watch-com-media",
"lint:js": "eslint --config build/eslint.config.mjs build administrator/components/com_media/resources/scripts",
"lint:testjs": "eslint --config build/eslint-tests.mjs tests/System",
"lint": "npx @biomejs/biome lint",
"lint:css": "stylelint --config build/.stylelintrc.json \"administrator/components/com_media/resources/**/*.scss\" \"administrator/templates/**/*.scss\" \"build/media_source/**/*.scss\" \"build/media_source/**/*.css\" \"templates/**/*.scss\" \"installation/template/**/*.scss\"",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the lint:css script option perhaps also be removed?
If I saw it properly, the command would no longer work either.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'll remove it eventually. Firstly need to port over the CSS ordering rules to Biome (if it's supported)

@C-Lodder
Copy link
Member Author

@laoneo Biome doesn't support .scss files, only .css, so shall I re-add the stylelint to cater for them?

@laoneo
Copy link
Member

laoneo commented Sep 27, 2025

Looks like then, otherwise there is a better alternative.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NPM Resource Changed This Pull Request can't be tested by Patchtester PR-6.0-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants