Skip to content

feat(plugin-js): per-package ImportGraph cache, js_lint driver - #365

Closed
raphaelvigee wants to merge 1 commit into
raphaelvigee/feat-plugin-js-m4-testfrom
raphaelvigee/feat-plugin-js-m5-lint
Closed

feat(plugin-js): per-package ImportGraph cache, js_lint driver#365
raphaelvigee wants to merge 1 commit into
raphaelvigee/feat-plugin-js-m4-testfrom
raphaelvigee/feat-plugin-js-m5-lint

Conversation

@raphaelvigee

@raphaelvigee raphaelvigee commented Aug 5, 2026

Copy link
Copy Markdown
Member

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

  • cargo build -p plugin-js -p plugin-js-cdylib
  • cargo test -p plugin-js
  • cargo clippy -p plugin-js -p plugin-js-cdylib --all-targets -- -D warnings
  • cargo fmt --check -p plugin-js -p plugin-js-cdylib

Stack created with GitHub Stacks CLIGive Feedback 💬

@raphaelvigee
raphaelvigee force-pushed the raphaelvigee/feat-plugin-js-m5-lint branch from bcf8663 to 279d191 Compare August 7, 2026 11:21
@raphaelvigee
raphaelvigee marked this pull request as ready for review August 7, 2026 12:57
@raphaelvigee
raphaelvigee force-pushed the raphaelvigee/feat-plugin-js-m5-lint branch from 279d191 to 3cf2668 Compare August 7, 2026 12:58
@raphaelvigee
raphaelvigee force-pushed the raphaelvigee/feat-plugin-js-m5-lint branch from 3cf2668 to 77211c6 Compare August 7, 2026 16:22
@raphaelvigee
raphaelvigee force-pushed the raphaelvigee/feat-plugin-js-m5-lint branch from 77211c6 to 9d4835d Compare August 7, 2026 18:31
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
@raphaelvigee
raphaelvigee force-pushed the raphaelvigee/feat-plugin-js-m5-lint branch from 9d4835d to 54dd04b Compare August 7, 2026 19:56
@raphaelvigee

Copy link
Copy Markdown
Member Author

Superseded by #370 — the whole stack was squash-collapsed into a single PR rebased on master, per request.

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