Repository navigation
fix(eslint): non-blocking refresh, config edits, rule options and suppression warnings - #255
Merged
Merged
Conversation
… 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.
Merged
This was referenced Oct 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
src/eslint/worker.tstrusted 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 inAtomics.wait. In an editor, that is every lint after a short pause.rulesandoverrides. So the main thread kept its old config: a rule turned off inonsystem.config.jsonor.mjswas still reported until the ESLint server restarted.schema: [], so'onsystem/no-hardcoded-color': ['error', { allow: ['#fff'] }]failed withshould NOT have more than 0 items.checkwarns about// onsystem-disable-next-line no-hardcoded-colour, but only real rule ids were bridged, so ESLint said nothing about it.--cache.What changed
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-shoteslintrun always loads fresh.versionis 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 therequire.cachedelete), since the worker reads configs throughloadTarget→loadConfig, so the worker needed no loader of its own. An edited.mjsis a new module, and now its rules reach the plugin.pnpm corpus eslint: one run out of three reporteddub-rootas DIFFERENT (oneprefer-design-system-componentfinding missing). The corpus lints every run in one process, and the worker kept projects by root only. Sodub-root's first lints could be answered from theapps/webproject 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.[{ allow: string[] }](strict, as inonsystem.config.json: an unknown key fails validation). ESLint'sallowtakes the place of the one the project's config gives that rule for the file, the way ESLint andoverridesreplace 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,warnin the recommended config. Carried by the first rule, the warning would have shown as an error of an unrelated rule (most rules default toerror, andno-hardcoded-colorruns first), andeslint-disableof 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.allowdoes), what each ESLint run pays for the plugin, withonsystem checkor the Action for a CI gate, and that--cachekeys on the file and ESLint's config, not the design system.onsystem/suppression, a new warning on upgrade (a run with--max-warnings 0over code with a misspelled suppression comment now fails), and the rules take options.How it was tested
New
test/eslint-bridge.test.ts:onsystem.config.json(rules) and inonsystem.config.mjs(overrides): fails on main (still reported when the 10 s poll ends).allowover the project's, anoverridesoff that stays off, and an unknown option rejected. Theallowcase fails on main withshould NOT have more than 0 items.check --format json, as a warning (fails on main).onsystem/suppressionfirst, on a project whose config is broken: "could not check" comes fromno-hardcoded-coloras an error, not fromsuppressionas a warning (failed on this PR's first head).loadTargetis wrapped in a countingvi.fn(avi.mockofsrc/workspace.jsthat keeps the real implementation), its call count is unchanged whenload()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 includessuppression.Editor simulation on dub, the finding's repro, not in the suite:
apps/webof dubinc/dub at the corpus sha 70c96f9 (fetched bypnpm corpus, never installed), one ESLint 10.12.0 instance with typescript-eslint's parser,lintTextonapp/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:[387, 364, 358, 356, 349, 353, 354, 352, 345, 366],[367, 384, 356, 352, 354, 347, 363, 352, 371, 365])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])[70, 58, 64, 59, 33, 48, 54, 48, 43, 57],[62, 64, 52, 52, 43, 53, 53, 51, 53, 34])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])[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(noschema.jsonchange) all pass.pnpm test:coveragepasses its thresholds.pnpm corpus: counts matchcorpus/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)
DesignSystemHost.watch()does, so that even the first lint after a change sees it. The watch logic is private toDesignSystemHostinsrc/design-system.ts, which is outside this PR's files.scripts/corpus.mjsstill dropssuppressionfrom check's side of the ESLint parity (notInEslint). Now that the plugin reports it, the parity can compare it too.--concurrencythreads, and load TypeScript lazily.settings, so that a design-system change invalidates.eslintcache.