feat(plugin-js): per-package ImportGraph cache, js_lint driver - #365
Draft
raphaelvigee wants to merge 1 commit into
Draft
feat(plugin-js): per-package ImportGraph cache, js_lint driver#365raphaelvigee wants to merge 1 commit into
raphaelvigee wants to merge 1 commit into
Conversation
M5 of the JS/TS plugin. Perf fix (prerequisite): deps_config, typecheck_config, and test_config each rebuilt the whole package's ImportGraph from scratch on every Provider::get call — for js_test, once per test file. Flagged independently by both the M2 and M4 reviews and deferred twice. Added a per-package memoized cache (keyed OnceCell behind a Mutex whose critical section is just the get-or-insert, so unrelated packages never serialize behind one lock) shared by all import-graph consumers, plus a call-count test proving it actually memoizes. Also added the equivalent cache for workspace-member discovery, the same O(P) redundant-walk shape found alongside it. js_lint: cacheable per-package target, oxlint default + eslint (with type-aware parserOptions.project support) via a single linter config option — same naming rule as every other driver here. Reuses the lockfile- driven third-party resolution and tsconfig-extends-chain handling already established for js_typecheck/js_test. Reviewed by feature-quality/code-quality/hermeticity — five BLOCKERs found and fixed: a fabricated package.json-config fallback that doesn't match how oxlint/eslint actually discover config and would break real invocations; an unconditional hard-fail on packages with zero lintable source files (now a clean no-op); only the first parserOptions.project entry in a multi-entry eslint config being tracked, silently dropping the rest from the cache key; an unvalidated parserOptions.project path letting a repo-controlled eslint config make heph read and hash an arbitrary host file (e.g. /etc/hostname) with no workspace-containment check — a real escape, now a hard error instead of a silent fallback; and eslint configs' own relative-path extends/imports (a shared base config) going untracked, the same class of gap already fixed once for js_test's runner config. Explicitly deferred: converging graph_cache/tsc_cache/testrunner_cache/ linter_cache onto the repo's existing hmemoizer primitive for panic containment across the ABI seam (pre-existing pattern, not a new regression, scoped as its own follow-up); a same-package concurrent-race test for the new cache (single-flight correctness argued from tokio's OnceCell contract, not yet proven under real concurrency); no bin-e2e coverage for any JS driver's dlopen/ABI-crossing seam yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3wZfyPsG8stfRQuybLRjN
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
1 participant
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.
M5 of the JS/TS heph plugin plan (part 5/6 of the stack).
Perf fix (prerequisite): deps_config, typecheck_config, and test_config each rebuilt the whole package's ImportGraph from scratch on every Provider::get call — for js_test, once per test file. Flagged independently by both the M2 and M4 reviews and deferred twice. Added a per-package memoized cache (keyed OnceCell behind a Mutex whose critical section is just the get-or-insert, so unrelated packages never serialize behind one lock) shared by all import-graph consumers, plus a call-count test proving it actually memoizes. Also added the equivalent cache for workspace-member discovery.
js_lint: cacheable per-package target, oxlint default + eslint (with type-aware parserOptions.project support) via a single linter config option. Reuses the lockfile-driven third-party resolution and tsconfig-extends-chain handling already established for js_typecheck/js_test.
Reviewed by feature-quality/code-quality/hermeticity — five BLOCKERs found and fixed: a fabricated package.json-config fallback that doesn't match how oxlint/eslint actually discover config; an unconditional hard-fail on packages with zero lintable source files; only the first parserOptions.project entry in a multi-entry eslint config being tracked; another real workspace escape — an unvalidated parserOptions.project path letting a repo-controlled eslint config make heph read and hash an arbitrary host file (e.g. /etc/hostname) with no workspace-containment check, now a hard error instead of a silent fallback; and eslint configs' own relative-path extends/imports going untracked.
Explicitly deferred: converging the several OnceCell-based caches onto the repo's hmemoizer primitive for panic containment across the ABI seam; a same-package concurrent-race test for the new cache; no bin-e2e coverage for any JS driver's dlopen/ABI-crossing seam yet.
Test plan
Stack created with GitHub Stacks CLI • Give Feedback 💬