Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use biome for JS linting #133

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Use biome for JS linting #133

wants to merge 15 commits into from

Conversation

syzygy333
Copy link
Contributor

Summary

Uses Biome in place of ESLint and Prettier (this reduces the number of packages needed and avoids having to adapt to breaking changes in new version of ESLint). To support this change, ddev config is also updated to use a newer version of Node and the package.lock also needed to be rebuilt and all the packages updated. The required packages in Composer were also updated.

Metadata

Question Answer
Did you use a meaningful pull request title? yes
Did you apply meaningful labels to the pull request? yes
Did you perform a self review first? yes
Documentation reflects changes? yes
CHANGELOG reflects changes? yes
Unit/Functional tests reflect changes? no
Did you perform browser testing? yes
Risk level Medium

@syzygy333 syzygy333 added documentation security dependencies Pull requests that update a dependency file labels Jul 29, 2024
@raubin
Copy link
Member

raubin commented Aug 1, 2024

@syzygy333 - I ran into errors when testing. I'm on an M1 Mac, so I occasionally see these arm64 version errors with packages.

When running ddev npm run polish:

> drupal-scaffold@1.0.0 polish
> npm run format && npm run lint


> drupal-scaffold@1.0.0 format
> npx @biomejs/biome format --write ./web/themes/custom/**/assets/js/*.es6.js

node:internal/modules/cjs/loader:1148
  throw err;
  ^

Error: Cannot find module '@biomejs/cli-linux-arm64/biome'
Require stack:
- /var/www/html/node_modules/@biomejs/biome/bin/biome
    at Module._resolveFilename (node:internal/modules/cjs/loader:1145:15)
    at Function.resolve (node:internal/modules/helpers:190:19)
    at Object.<anonymous> (/var/www/html/node_modules/@biomejs/biome/bin/biome:51:11)
    at Module._compile (node:internal/modules/cjs/loader:1358:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1416:10)
    at Module.load (node:internal/modules/cjs/loader:1208:32)
    at Module._load (node:internal/modules/cjs/loader:1024:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:174:12)
    at node:internal/main/run_main_module:28:49 {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ '/var/www/html/node_modules/@biomejs/biome/bin/biome' ]
}

I needed to install biome from scratch or delete the lock file to get the right architecture support:

npm install --save-dev --save-exact @biomejs/biome

This seems to track with this known issue: npm/cli#4828

Since this is a scaffold project, I think we should just delete the lock file so that the install can build for the user's platform.

The next error I encountered is that there's no custom theme in scaffold and the biome config expects that, so this might be something we want to document.

> drupal-scaffold@1.0.0 polish
> npm run format && npm run lint


> drupal-scaffold@1.0.0 format
> npx @biomejs/biome format --write ./web/themes/custom/**/assets/js/*.es6.js

./web/themes/custom/**/assets/js/*.es6.js internalError/io  INTERNAL  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ No such file or directory (os error 2)

  ⚠ This diagnostic was derived from an internal Biome error. Potential bug, please report it if necessary.

Formatted 0 files in 296µs. No fixes applied.
internalError/io ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ No files were processed in the specified paths.


Failed to run npm run polish: exit status 1

package-lock.json Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file documentation security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants