You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 520dd77
Browse filesBrowse the repository at this point in the historyBrowse files
perf: Implement fast paths in critical areas (#21210)
* chore: Improve CLI perf
* fix: revert lazy-getter exports in lib/api.js, drop unused export
The lib/api.js lazy-getter rewrite broke Node's CJS-to-ESM named-export
interop: cjs-module-lexer cannot statically detect getter-based exports,
so `import { RuleTester } from "eslint"` (and ESLint/SourceCode/Linter)
threw a SyntaxError at runtime for ESM consumers. This was caught by the
ecosystem plugin tests (@eslint/css, @eslint/json, @eslint/markdown,
eslint-plugin-unicorn) and the "Are the types wrong?" check, none of
which are exercised by this repo's CJS-only unit test suite. Reverting
to eager top-level requires restores the Loading benchmark to baseline,
but that's the correct tradeoff over breaking every ESM consumer of the
package.
Also drop the `attributeError` export from source-code-visitor.js: it's
only used internally within that module now, and knip correctly flagged
it as an unused export.
* perf: lazily construct the shared ajv instance in Config
ajv pulls in ~40 modules that are only needed once a rule's schema
actually needs validating. Constructing it lazily on first use (instead
of eagerly at module load time) recovers most of the Loading benchmark
improvement lost when lib/api.js reverted to eager requires, without
touching the public export surface: this is purely internal to
lib/config/config.js.
* fix: address review feedback on CLI perf changes
Correctness fixes from PR review, each with a regression test that fails
without the corresponding fix:
- stylish: the `stringLength()` fast path only checked for the 7-bit ESC
introducer, but `util.stripVTControlCharacters()` also strips sequences
introduced by the 8-bit CSI character (U+009B). A message containing
only CSI sequences had its control bytes counted as visible characters,
misaligning table columns. Check both introducers.
- linter/source-code-visitor: the direct-listener fast path tags the
listener function object with its rule ID, but two rules can register
the same function object (for example, a shared visitor returned from a
helper). The second registration overwrote the first rule's tag, so
errors thrown from the first rule were attributed to the second. Fall
back to the wrapper when a function is already tagged for a different
rule.
- source-code-visitor: `attributeError()` preserved a pre-existing
`ruleId` on the thrown error, but the wrapper it replaced always
overwrote it with the rule whose listener threw. Assign unconditionally
to keep error attribution unchanged.
Also make `ajv` lazy in RuleTester. `lib/api.js` eagerly requires
RuleTester, so the eager `ajv` construction there pulled ~40 modules into
the dependency graph of every consumer of the `eslint` package. Together
with the existing lazy `ajv` in Config (both are required, since
api.js -> linter -> config is a separate path to it), this cuts the
`require("eslint")` graph from 177 modules to 131.
* test: add unit tests for SourceCodeVisitor error attribution
The `ruleId` overwrite behavior change was caught in review rather than by
a failing test, because `callSync()` had no direct coverage of error
propagation at all -- the attribution contract lives in this module but was
only exercised indirectly through `Linter`.
Adds unit tests covering the contract directly: an error from an untagged
function propagates without a `ruleId`, an error from a tagged function is
attributed to that rule, an existing `ruleId` is overwritten by the rule
that actually threw, attribution works on both the single-argument fast
path and the multi-argument path, and functions registered after a thrower
are not called.
The overwrite tests fail against the previous preserve-if-unset behavior.
* fix: use a WeakMap for listener rule IDs, address review feedback
Replaces the symbol tag on listener functions with a module-scoped
`WeakMap`. Tagging wrote a property onto a function object owned by the
rule, which the rule's author has no reason to expect; a `WeakMap` keeps
that association external. The hot path is unaffected either way, since
the mapping is only consulted when a listener throws.
This also lets the `Object.isExtensible()` guard go away: a `WeakMap` can
key a frozen function, so frozen listeners now take the fast path instead
of falling back to a wrapper. Added tests covering a frozen listener and a
non-function listener value, which are the two remaining conditions on
that branch.
Guards the `ruleId` assignment against non-object thrown values. Rules can
throw primitives, and assigning to one throws a strict-mode `TypeError`
that replaces whatever the rule actually threw. Note this only fixes the
assignment in this module -- `SourceCodeTraverser` and
`_verifyWithFlatConfigArray` decorate thrown values the same way and still
mask primitives, which is pre-existing behavior left alone here.
Moves the lazy `ajv` state and helper in `Config` and `RuleTester` out from
between the `require` calls and into the "Private Members" and "Helpers"
sections.
* perf: route all rule lookups in Config through the cache
`#normalizeRulesConfig()` and `validateRulesConfig()` called
`getRuleFromConfig()` directly, bypassing the cache added for
`getRuleDefinition()`. Since both run over every configured rule, each rule
was looked up three times -- repeating the rule ID parsing and the plugin
proxy traps each time.
Routing them through `getRuleDefinition()` leaves `getRuleFromConfig()` with
a single caller, so the cache is the only lookup path. With all core rules
enabled, `getRuleFromConfig()` executions during a lint drop from 876 to 292
(one per rule instead of three).
* perf: don't materialize non-array traversal steps
Addresses review feedback on #21210.
`traverseSync()` converted a non-array `steps` iterable with `Array.from()`
so it could iterate by index. That is slower than just iterating the
iterable, and it fully materializes a lazy one. Measured over 50k steps on
Node v24.18.1: an array iterator went from 0.817 ms to 2.198 ms and a
generator from 2.353 ms to 3.550 ms. This is not hypothetical --
`@eslint/markdown` returns `steps.values()` from `traverse()`, so every
markdown file paid for a full copy of the steps array.
Arrays are still iterated by index; everything else now uses `for...of`.
The step body moved into a local function so both loops can share it,
which keeps most of the win on the array path (0.262 ms vs 0.165 ms
inlined, against 0.879 ms for `for...of`). An interleaved A/B lint of
`tests/performance/jshint.js` with all core rules shows no measurable
change (3.385 s vs 3.340 s, within noise).
`createFastMatcher()` also drops three checks that couldn't affect it:
* `left.subject` / `right.subject` / `field.subject` -- `esquery` only
reads `subject` in its `sibling` and `adjacent` matchers and in
`esquery.query()`, none of which are reachable here, so a subject
indicator has no effect on `esquery.matches()` for a `child` selector.
These made the fast path bail on selectors it handles identically.
* the `compound[field, wildcard]` branch -- unreachable, because `*` is a
legal identifier character in `esquery`'s grammar, so `.field*` parses
as a field named `field*`. An exhaustive check of every 4-token
selector over a representative alphabet parsed 10,282 selectors and
produced no such compound.
Adds tests asserting that the fast matcher agrees with `esquery.matches()`
across these selector shapes, and that non-array steps are consumed one at
a time rather than all at once.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C4gazgTwRgBSJKkYRQdhW3
* Add comment about string optimization
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
0 commit comments