Skip to content

feat(frameworks): add shared Next.js detector pipeline + enforced next lint (TS/JS)#414

Draft
MacHatter1 wants to merge 2 commits intopeteromallet:mainfrom
MacHatter1:hatter/nextjs-framework-depth
Draft

feat(frameworks): add shared Next.js detector pipeline + enforced next lint (TS/JS)#414
MacHatter1 wants to merge 2 commits intopeteromallet:mainfrom
MacHatter1:hatter/nextjs-framework-depth

Conversation

@MacHatter1
Copy link
Contributor

PR: Next.js framework smell depth + shared framework detection + JS parity

Summary

Before this branch, Desloppify had no dedicated Next.js framework detector layer. Next.js repos were only evaluated by generic TypeScript/Javascript detectors (smells, coupling, security, etc.).

This PR adds a shared TypeScript framework detection layer and deep Next.js framework smell scanning, then reuses that stack for JavaScript so Next.js issues are detected consistently across TS/JS projects.

It also integrates next lint as a framework-level quality gate: when Next.js is primary and lint cannot run, we emit a next_lint::unavailable issue instead of silently skipping.

Root Cause

  1. There were no explicit Next.js-specific detector checks before this work; framework misuse was effectively invisible unless it happened to be caught by generic TS/JS detectors.
  2. Framework detection for external scan paths could bind to the workspace root and miss the scanned repo’s package.json.
  3. use_server_not_first produced false positives for valid inline server actions.

What Changed

Shared framework layer

  • Added shared TS framework detection package:
    • desloppify/languages/typescript/frameworks/*
  • Added Next.js framework info + shared phase orchestration:
    • desloppify/languages/typescript/frameworks/nextjs/info.py
    • desloppify/languages/typescript/frameworks/nextjs/phase.py
    • desloppify/languages/typescript/frameworks/nextjs/scanners.py

Next.js lint integration (enforced behavior)

  • Added framework node helper:
    • desloppify/languages/_framework/node/next_lint.py
  • Behavior:
    • Runs npx --no-install next lint --format json.
    • If lint is unavailable/unparseable, emits next_lint::unavailable (failing issue).

JavaScript parity

  • Added JS Next.js phase:
    • desloppify/languages/javascript/phases_nextjs.py
  • Inserted phase into JS plugin pipeline:
    • desloppify/languages/javascript/__init__.py

Detector and scoring wiring

  • Registered nextjs and next_lint in detector catalog and display ordering.
  • Added file-based scoring policy entries for both detectors.

Scanner depth (Next.js)

Includes high-value checks for:

  • Server-only imports in client modules
  • Server-only exports from client modules
  • Pages APIs under app/
  • next/navigation usage in pages/
  • Client env leakage (process.env non-NEXT_PUBLIC_*)
  • Route handler/middleware misuse
  • next/head misuse in App Router
  • next/document misuse
  • Missing use client where needed
  • Client layout and async-client smells
  • Directive placement checks

False-positive fixes

  • Fixed module-level use server placement logic so valid nested inline server actions are not flagged.

External path detection fix

  • Framework detection now properly resolves nearest package.json for external scan targets.

Impacted Shared Surfaces

  • Framework detection core:
    • desloppify/languages/typescript/frameworks/detect.py
  • TS smells phase integration point:
    • desloppify/languages/typescript/phases_smells.py
  • Generic discovery helper additions:
    • desloppify/base/discovery/source.py
  • Detector registry/scoring:
    • desloppify/base/registry/catalog_entries.py
    • desloppify/base/registry/catalog_models.py
    • desloppify/engine/_scoring/policy/core.py

Why This Is Systemic

  • The fix centralizes framework detection and Next.js smell mapping into shared abstractions, then reuses them in both TS and JS language plugins.
  • It avoids per-framework/per-language hardcoding at the callsite level.
  • It enforces lint participation through a detector issue path rather than optional logging.

Regression Protection

Added/updated tests

  • desloppify/languages/typescript/tests/test_ts_nextjs_framework.py
    • framework primary detection
    • external scan path detection
    • broad Next.js smell coverage
    • nested inline server-action false-positive guard
  • desloppify/languages/javascript/tests/test_js_nextjs_framework.py
    • JS plugin phase insertion
    • JS Next.js smell emission
  • Updated supporting tests for policy/runtime expectations:
    • desloppify/tests/review/work_queue_cases.py
    • desloppify/languages/typescript/tests/test_ts_unused.py

Test result

  • 6051 passed, 145 skipped

OSS Benchmark (baseline vs this branch)

All scans run with: --lang typescript --profile objective --skip-slow --no-badge

Important context: baseline has nextjs=0 in these comparisons because baseline did not include a Next.js-specific detector path.

Control repos (no expected Next.js smells)

  1. vercel/next-app-router-playground
    • baseline: nextjs=0, new: nextjs=0
  2. vercel/nextjs-subscription-payments
    • baseline: nextjs=0, new: nextjs=0

Issue-bearing repos

  1. shadcn-ui/taxonomy
    • baseline: nextjs=0
    • new: nextjs=1
    • finding: mixed routers (app/ + pages/) project smell
  2. descope-sample-apps/linkedin-sample-app
    • baseline: nextjs=0
    • new: nextjs=2
    • findings:
      • App Router page imports next/head
      • App Router root layout is client component ("use client")

Notes

  • This branch is ready for PR creation after your review of this draft.

Copy link
Owner

@peteromallet peteromallet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Well-structured feature with a solid architecture (types → catalog → detect → scanners → phase). The layered separation will make it easy to add frameworks beyond Next.js. That said, there are some correctness issues and the PR is large enough that splitting would help.

Architecture strengths

  • Clean layered separation: contracts → data-driven signatures → generic scoring → framework-specific adapters
  • Shared _framework/node/ location for cross-language Node tooling
  • Caching via lang.review_cache avoids redundant detection

Correctness issues (high priority)

  1. scan_nextjs_use_server_in_client false-positives on valid inline server actions — a 'use client' module is allowed to contain functions with 'use server' inside the function body (inline server actions, valid Next.js 14+). The scanner flags any 'use server' presence in a 'use client' module. The PR already correctly handles the reverse case in _find_misplaced_module_use_server_directive with indentation awareness but doesn't apply the same logic here.

  2. _has_use_server_directive_anywhere matches comments/string literals_USE_SERVER_LITERAL_RE matches the raw string anywhere on a line, so // don't use 'use server' here or const x = "use server" would trigger false positives.

  3. Error/middleware scanners miss .js/.jsx extensionsscan_nextjs_error_files_missing_use_client targets only .ts/.tsx but is shared with the JS plugin. Same issue for _is_middleware. JS Next.js projects won't get these checks.

Code quality (medium priority)

  1. 572-line phase.py is copy-paste — ~25 identical blocks (call scanner, iterate entries, make_issue(), log). A data-driven approach (list of scanner configs with id/tier/confidence/summary-template) could cut this to ~50 lines and eliminate copy-paste bugs.

  2. JS plugin hard-coupled to TS plugin internalsjavascript/phases_nextjs.py imports detect_primary_ts_framework directly from the TS package. The detection logic is language-agnostic. Consider moving shared framework detection to languages/_framework/ or at least dropping _ts_ from the name.

  3. Hardcoded framework IDs in generic scoringdetect.py:100-108 has if sig.id == "nextjs" / elif sig.id == "vite" inside what should be a generic function. Should be a field on FrameworkSignature (e.g., script_pattern).

  4. Dead Vite signaturecatalog.py:39-49 defines a Vite signature with no scanners, no registry entry, no tests. Forward-looking scaffolding adds confusion.

Missing coverage

  1. No unit tests for run_next_lint — JSON extraction, relativization, and error handling paths untested (only monkeypatched away).

  2. No --skip-slow integration for next lint — 180s subprocess timeout with no check for the existing --skip-slow flag.

Suggestion: split the PR

This is ~2800 new lines across 23 files bundling generic framework infra + Next.js scanners + next lint + JS parity + a tsc fallback chain change. Consider:

  • PR A: Generic framework detection layer (types, catalog, detect, scoring wiring)
  • PR B: Next.js scanners + phase + next lint + tests
  • PR C: The tsc fallback chain change in unused.py (unrelated)

Good work on the overall design — the issues above are mostly about tightening the implementation. The false-positive items (#1-3) are the most important to address before merge.

@peteromallet
Copy link
Owner

Hey @MacHatter1 — nice work on the architecture here, the layered separation (types → catalog → detect → scanners → phase) is solid and will scale well to other frameworks.

There are a few things to address before we can merge:

Must fix (correctness):

  1. scan_nextjs_use_server_in_client flags valid inline server actions — a 'use client' module is allowed to have 'use server' inside function bodies (Next.js 14+). Needs the same nesting awareness you already have in _find_misplaced_module_use_server_directive.
  2. _has_use_server_directive_anywhere matches comments/string literals — should use _code_text or _strip_ts_comments like the other scanners.
  3. Error/middleware scanners only target .ts/.tsx but are shared with the JS plugin — JS Next.js projects won't get error.js/global-error.js or middleware checks.

Should fix:
4. The 572-line phase.py is 25 copy-paste blocks of the same pattern (call scanner → iterate → make_issue → log). A data-driven approach with a list of scanner configs would cut this dramatically and eliminate copy-paste risk.
5. javascript/phases_nextjs.py imports detect_primary_ts_framework directly from the TS package — the detection logic is language-agnostic, so move it to languages/_framework/ or rename to drop _ts_.
6. Hardcoded if sig.id == "nextjs" in the generic detect.py scoring function — should be a field on FrameworkSignature.
7. Dead Vite signature in catalog.py with no consumers — remove until needed.
8. next lint (180s subprocess) doesn't check --skip-slow.

Let me know if you have questions on any of these!

@peteromallet
Copy link
Owner

@MacHatter1 — one more thing I wanted to share after thinking through the architecture more carefully. This isn't blocking the correctness fixes above, but I think it's worth discussing before we merge because it affects how future framework detectors would be built on top of this.

The core idea is great

Framework-specific detection is a real gap right now. A Next.js project has a whole class of issues (misplaced directives, missing use client on error boundaries, server/client boundary violations) that generic TS/JS detectors will never catch. You've identified the right problem and the scanner coverage is genuinely useful.

The factoring concern

The current approach puts the framework detection layer inside the TypeScript plugin (typescript/frameworks/), then has the JavaScript plugin reach across and import from it (detect_primary_ts_framework, nextjs_info_from_detection, etc.). Today, no language plugin imports from another language plugin — that's been a clean boundary, and breaking it creates a coupling where changes to TS internals can silently break JS.

The reason this happens is that framework detection ("is this a Next.js project?") is actually language-agnostic — it reads package.json, checks for dependencies and config files. That part doesn't belong in either language plugin.

Suggested factoring

Something like:

_framework/
  node/                              # Already exists
    frameworks/
      types.py                       # FrameworkSignature, FrameworkDetection
      catalog.py                     # Next.js, Vite signatures (data-driven)
      detect.py                      # Generic package.json scoring

languages/
  typescript/
    frameworks/
      nextjs.py                      # TS-specific scanners (.tsx patterns)
  javascript/
    frameworks/
      nextjs.py                      # JS-specific scanners (.jsx patterns)

Detection goes in _framework/node/frameworks/ — both TS and JS import from there. Language-specific scanners stay in their respective plugins. No cross-plugin imports.

This also naturally solves the detect_primary_ts_framework naming issue — it'd just be detect_primary_framework in a shared location.

The phase.py pattern

The other thing worth considering: the 572-line phase.py with 25 structurally identical blocks (call scanner → iterate → make_issue → log) is essentially doing what generic_support already does with tool specs — mapping tool output to issues via config. A data-driven approach where each scanner is a config entry (id, tier, confidence, summary template) iterated in a single loop would be much more maintainable and is closer to how the rest of the codebase works.

Summary

None of this takes away from the quality of the scanners themselves or the detection logic — those are solid. It's really just about where things live so that future framework detectors (React, Vue, Angular, etc.) can follow the same pattern cleanly.

Happy to chat through any of this if it's helpful!

@peteromallet
Copy link
Owner

@MacHatter1 — I opened #418 to capture some longer-term architectural thinking about how framework detection should work at scale, building on what you've started here.

The TL;DR: your scanners and detection logic are great, but we think framework detection should follow the tree-sitter pattern — spec-driven, data-driven, working as a horizontal layer across both shallow and deep plugins. That way adding a new framework is just adding a spec file, and both TS and JS get coverage from the same spec automatically (no cross-plugin imports needed).

Would love your thoughts on it since you've done the actual work of building the first framework detector and know best what the scanner authoring experience needs to look like. The issue has the full reasoning and a concrete proposed structure.

@MacHatter1 MacHatter1 marked this pull request as draft March 13, 2026 19:34
@MacHatter1
Copy link
Contributor Author

@peteromallet I've converted this PR to a draft for now until a way forward from #418 is reached.

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.

2 participants