Skip to content

fix(eslint): non-blocking refresh, config edits, rule options and suppression warnings - #255

Merged
dgesteves merged 3 commits into
mainfrom
fix/os2-45-eslint-refresh-options
Oct 29, 2026
Merged

dgesteves merged 3 commits into
mainfrom
fix/os2-45-eslint-refresh-options

Conversation

@dgesteves

@dgesteves dgesteves commented Oct 29, 2026 •

Copy link
Copy Markdown
Owner

Roadmap item: OS2-45 (findings os-perf-pkg/eslint-refresh-blocks-editor, the ESLint half of os-bugs-edges/os-stale-config-long-running, os-arch/os-eslint-bridge-gaps, os-perf-pkg/eslint-cache-stale, and the docs part of os-perf-pkg/eslint-plugin-overhead).

What was wrong

  • Every lint after a pause waited for a refresh. src/eslint/worker.ts trusted what it had loaded for 2 s. After that, the next lint re-ran project detection, the model's cache check and the component glob plus a realpath per file, while ESLint's thread sat in Atomics.wait. In an editor, that is every lint after a short pause.
  • Config edits never reached a running ESLint. The version the plugin compares was the model's cache key, which leaves out rules and overrides. So the main thread kept its old config: a rule turned off in onsystem.config.json or .mjs was still reported until the ESLint server restarted.
  • Rule options crashed ESLint's config validation. Every rule had schema: [], so 'onsystem/no-hardcoded-color': ['error', { allow: ['#fff'] }] failed with should NOT have more than 0 items.
  • Suppression typos were dropped. check warns about // onsystem-disable-next-line no-hardcoded-colour, but only real rule ids were bridged, so ESLint said nothing about it.
  • docs/eslint.md said nothing about any of this, about the plugin's cost, or about --cache.

What changed

  • Stale-while-revalidate in the worker. The worker answers each lint from what it holds. When that is 2 s old, the lint starts a look in the background: the target (project and config) first, then the file's project. The lints after the look get what it found. The look first compares the design system's fingerprint (fingerprint(), the one the MCP server's watcher uses), so when nothing it is built from changed, the model and component list are kept and no cache file is read. The look starts on a later turn of the event loop (setImmediate), after the lint that started it has its answer: its first part (finding the project, reading the config, the fingerprint) is synchronous, and started at once it held up that answer. That lint is answered from what was loaded before, so after a design-system or config change the file shows its earlier findings until it is linted again (the next edit, or the next save when the editor lints on save); docs/eslint.md says so. A one-shot eslint run always loads fresh.
  • Config hash in the version. version is now a hash of the model's key and the whole resolved config, rules included. The JS/TS config already goes through fix(server): retry failed loads, reload JS configs, answer per path when --root holds projects #242's content-hash loader (readConfigFile, ?v=<sha256> plus the require.cache delete), since the worker reads configs through loadTarget → loadConfig, so the worker needed no loader of its own. An edited .mjs is a new module, and now its rules reach the plugin.
  • Projects are kept per target and root. I found this through pnpm corpus eslint: one run out of three reported dub-root as DIFFERENT (one prefer-design-system-component finding missing). The corpus lints every run in one process, and the worker kept projects by root only. So dub-root's first lints could be answered from the apps/web project an earlier run had built with its own config. Main has the same latent bug inside its 2 s window (a new test, "keeps two configs of the same project apart", fails on main too). Keying by target and root fixes it. After that, the full parity passed in 3 of 3 runs.
  • A failed load is kept until the next look, as before. A broken config is not reloaded on every lint; the lints until the next look say why, and a fix is picked up after the next look.
  • Rule options. Each rule takes [{ allow: string[] }] (strict, as in onsystem.config.json: an unknown key fails validation). ESLint's allow takes the place of the one the project's config gives that rule for the file, the way ESLint and overrides replace options. A rule the project's config turns off for a file stays off. A rule with options in ESLint's config is checked on its own pass, so it costs one more parse of the file; rules without options share the one check, as before.
  • onsystem/suppression. The finding proposed carrying suppression problems through the first rule that runs, as the "could not check" message is. I used a rule of its own instead, warn in the recommended config. Carried by the first rule, the warning would have shown as an error of an unrelated rule (most rules default to error, and no-hardcoded-color runs first), and eslint-disable of that rule would have hidden it. As its own rule, it has check's severity, message and position, and can be turned off. When a config lists it first, it does not carry "onsystem could not check this file": the first of the other rules does.
  • docs/eslint.md: how the background look works, where rule options live (and what an ESLint allow does), what each ESLint run pays for the plugin, with onsystem check or the Action for a CI gate, and that --cache keys on the file and ESLint's config, not the design system.
  • A minor changeset: the recommended config gains onsystem/suppression, a new warning on upgrade (a run with --max-warnings 0 over code with a misspelled suppression comment now fails), and the rules take options.

How it was tested

  • New test/eslint-bridge.test.ts:

    • Turning a rule off between two lints, in onsystem.config.json (rules) and in onsystem.config.mjs (overrides): fails on main (still reported when the 10 s poll ends).
    • A config that breaks, then is fixed, in one process.
    • Two configs of one project in one process (fails on main).
    • ESLint allow over the project's, an overrides off that stays off, and an unknown option rejected. The allow case fails on main with should NOT have more than 0 items.
    • A misspelled suppression rule, the same as check --format json, as a warning (fails on main).
    • A config that lists onsystem/suppression first, on a project whose config is broken: "could not check" comes from no-hardcoded-color as an error, not from suppression as a warning (failed on this PR's first head).
    • The worker on its own (Date faked): ten loads after 2.5 s pauses on a design system of 400 components. Each must answer with the version it holds before the look it started has begun: loadTarget is wrapped in a counting vi.fn (a vi.mock of src/workspace.js that keeps the real implementation), its call count is unchanged when load() resolves, and one higher once the look ends. No timing in the suite could see the synchronous start of the look; this order check failed on this PR's first head and passes now. A change is answered with the old version, then with the new one after the look. A config edit that only changes rules keeps the model and sends the new config.
  • test/eslint.test.ts: the existing refresh test now expects the lint after the pause to be answered from the held design system, then the change after the look (it failed on main for that reason). The rule list includes suppression.

  • Editor simulation on dub, the finding's repro, not in the suite: apps/web of dubinc/dub at the corpus sha 70c96f9 (fetched by pnpm corpus, never installed), one ESLint 10.12.0 instance with typescript-eslint's parser, lintText on app/app.dub.co/(dashboard)/[slug]/(ee)/program/commissions/create-commission-sheet.tsx, 1 + 10 calls with 2.5 s pauses. Per lint after each pause, two runs each, on this machine:

    • main: 345 to 387 ms ([387, 364, 358, 356, 349, 353, 354, 352, 345, 366], [367, 384, 356, 352, 354, 347, 363, 352, 371, 365])
    • this PR's first head (look started inside load()): 200 to 237 ms ([222, 233, 222, 217, 224, 236, 214, 200, 232, 232], [226, 233, 236, 233, 235, 221, 232, 210, 216, 237])
    • this branch: 33 to 70 ms ([70, 58, 64, 59, 33, 48, 54, 48, 43, 57], [62, 64, 52, 52, 43, 53, 53, 51, 53, 34])
    • main with the refresh turned off (FRESH_MS = 1e9), the floor: 31 to 100 ms ([45, 69, 58, 53, 52, 56, 54, 49, 31, 53], [100, 59, 47, 57, 54, 42, 46, 49, 55, 53])
    • the parser alone, without onsystem: 15 to 47 ms ([27, 24, 22, 21, 19, 19, 18, 15, 18, 16], [47, 37, 33, 37, 24, 34, 37, 27, 25, 33])

    Every onsystem build reported the same 133 messages. The 1-minute load average was 1.7 to 8.1 during the runs. A lint after a pause now costs what it costs with the refresh off.

  • Gate: pnpm install --frozen-lockfile --prefer-offline, pnpm format:check, pnpm lint, pnpm typecheck, pnpm test (106 files, 1370 passed, 3 skipped), pnpm build (no schema.json change) all pass. pnpm test:coverage passes its thresholds. pnpm corpus: counts match corpus/snapshot.json, no diff. pnpm corpus eslint: the plugin reports what check reports in all 25 runs. The branch is merged with main (fix(tokens): export round trip: DESIGN.md spacing steps, typography aliases, no DTCG duplicates #254).

  • Not run here: pnpm pack-test (it installs from the npm registry); CI runs it, including ESLint 8.57.

Follow-ups (not in this PR)

  • The worker could watch the design system's files, as the MCP server's DesignSystemHost.watch() does, so that even the first lint after a change sees it. The watch logic is private to DesignSystemHost in src/design-system.ts, which is outside this PR's files.
  • scripts/corpus.mjs still drops suppression from check's side of the ESLint parity (notInEslint). Now that the plugin reports it, the parity can compare it too.
  • The rest of os-perf-pkg/eslint-plugin-overhead: build the check from ESLint's parse, share one model per process across --concurrency threads, and load TypeScript lazily.
  • Optionally, put the model fingerprint into the recommended config's settings, so that a design-system change invalidates .eslintcache.
  • The worker keeps projects per ESLint working directory, settings and root, so two working directories that reach one root with the same config hold two models, and neither map is ever cleared (main keeps them forever too). Keying projects by root plus the config's fingerprint without its rules would share one model between them.

… config edits, rule options, suppression typos

The plugin's loader made ESLint wait for a new look at the project each
time what it held was two seconds old, so every lint after a pause in an
editor paid for that look. It now answers from what it holds and looks
again in the background; the lints after the look get what it found.

The version the plugin compares now covers the whole config, so a rule
turned off in the rules or overrides of onsystem.config.json or .mjs
stops being reported without a restart. Projects are kept per target,
so two configs of one project in one process do not answer for each
other.

Rules take [{ allow: string[] }] in ESLint's config, over the project's
allow for that rule, and onsystem/suppression reports a misspelled rule
in a suppression comment as a warning, as check does. docs/eslint.md
says where rule options live, what each ESLint run pays for the plugin,
and that --cache does not see design-system changes.

OS2-45
The look a lint starts ran its synchronous first part (finding the
project, reading its config, the design system's fingerprint) before
load() replied, so on dub each lint after a pause still waited for it.
It now starts on a later turn of the event loop. The worker test counts
loadTarget calls to check that no part of the look runs before the
answer.

onsystem/suppression no longer reports "onsystem could not check this
file" when a config lists it first: the other rules say so.

The changeset is minor: the recommended config gains a warning rule,
and the rules take options. docs/eslint.md says that the lint that
starts a look shows the earlier findings until the file is linted again.
@dgesteves
dgesteves merged commit 3a1ba6c into main Oct 29, 2026
19 checks passed
@dgesteves
dgesteves deleted the fix/os2-45-eslint-refresh-options branch October 29, 2026 10:14
@dgesteves-release dgesteves-release Bot mentioned this pull request Oct 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant