Opened 7 weeks ago
Closed 6 weeks ago
#36619 closed Cleanup/optimization (fixed)
Vendor eslint configuration dependencies to permit pre-commit usage without npm install
| Reported by: | Jacob Walls | Owned by: | Jacob Walls |
|---|---|---|---|
| Component: | Core (Other) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
The pre-commit hook for eslint has been broken for over a year. Noticed contributors at the DCUS 2025 sprints stumbling on this.
Eventually, we may be looking at another JS formatter, like Biome. Today, I'm interested in just fixing the hook.
More context is here, but in essence, the v9 eslint flat config format assumes you're a javascript project in the habit of running npm install. We're not; we only run npm install to run the js_tests.
We can either document that npm install is necessary to run pre-commit (and consider that objectors might prefer to remove the hook altogether instead of tolerating that), or we can write a local hook that fiddles with symlinks, or as I'm suggesting, just vendor the rule configuration and global definitions.
Sarah also suggested vendoring them here.
On vendoring:
eslint exports @js/recommended and @js/all specifically so that @js/all can iterate faster with every minor release; so vendoring @js/recommended doesn't seem horrible -- it's supposed to be stable.
Vendoring the globals creates a larger file, but there is various tweaking we can do there, like weighing whether it's better or mysterious to just vendor the constants we need ("browser" and "commonjs").
Vendoring isn't ideal, but I think it makes sense for us given that most contributors rarely need to run the js_tests.
From eslint blog:
all that was left was for environments to manage global variables ... so we are handing this responsibility back to you.
Change History (6)
comment:1 by , 7 weeks ago
| Description: | modified (diff) |
|---|---|
| Has patch: | set |
comment:2 by , 7 weeks ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:3 by , 7 weeks ago
| Patch needs improvement: | set |
|---|
comment:4 by , 7 weeks ago
| Patch needs improvement: | unset |
|---|
comment:5 by , 6 weeks ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
PR