mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-17 05:07:37 +08:00
fix(frameworks): detection reads an app's package.json three or more levels down (#2395)
declaredDependencies read the root manifest and the ones one or two directories down. An app whose package.json sits deeper was never read: an ASP.NET solution's src/Web/ClientApp/, or prometheus's web/ui/mantine-ui/ under a workspace root that declares only tooling. Every resolver gated on dependsOn() then stayed off for it: Angular Router and the Angular template pass, React Router's navigation, Vue Router, SvelteKit and TanStack Router. The existing pass is unchanged. When it leaves slots open, a second pass probes the directories three or more levels down that hold indexed JS/TS code, shallowest first and sorted within a level, at most 192 of them, never inside node_modules. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -64,6 +64,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
- `codegraph sync` and the file watcher now link an import to the file it names when that file is added after the importing file was indexed, or comes back after being deleted: `import 'package:app/b.dart';` in Dart, `import './side'` in JavaScript or TypeScript, `#include "net/socket.h"` in C and C++, `require 'inc/db.php'` in PHP, `import pkg.mod` in Python, a Lua `require`, or a C# `using` of a namespace declared later. Before, the import stayed unlinked until the importing file itself changed or the project was re-indexed, so `codegraph affected` and the files listed as using the new file missed it. Re-index after upgrading.
|
||||
- JSON files in a `templates` or `sections` folder are now read as Shopify Liquid only inside a Shopify theme, where the folder that contains `templates` or `sections` also has `layout/theme.liquid` or `config/settings_schema.json`. Before, all such JSON was, so a .NET project template's `template.json`, a schematic or a CMS's content showed up in the index as an empty Liquid file, and one that lists `sections` could be linked to an unrelated Liquid file of the same name. Shopify themes, including one kept in a subfolder of a repository, are indexed as before, and the next sync removes JSON that an earlier version indexed this way.
|
||||
- In C, C++ and Objective-C, a struct or class member declared as a pointer, reference, array or function, like `SharedState* shared;`, `jv elements[];` or `virtual Status Put(…) = 0;`, no longer makes the struct look like it inherits from the member's type. Before, `codegraph_explore` and impact listed those types as base classes, every struct holding a pointer to a type as one of its subclasses, and a method of that struct as an override of the type's method with the same name. Real base classes like `class Derived : public Base`, Objective-C superclasses and Go's embedded fields are unchanged. Re-index C, C++ and Objective-C projects after upgrading.
|
||||
- Framework support now switches on when an app's `package.json` sits three or more folders deep, as with an ASP.NET solution's Angular or React app in `src/Web/ClientApp/`, or a React app in a workspace folder like `web/ui/mantine-ui/`. Before, only the project root and the first two folder levels were read, so frameworks such as Angular, React Router, Vue Router, SvelteKit and TanStack Router could go undetected there: an Angular app's routes, its `routerLink` and `router.navigate(…)` navigation and its template's event bindings were missing, so a handler used only from a `(click)` binding had no callers, and a React app's `navigate('/login')` calls didn't reach their route. Re-index projects laid out this way after upgrading.
|
||||
|
||||
## [1.6.2] - 2026-10-03
|
||||
|
||||
|
||||
@@ -0,0 +1,246 @@
|
||||
/**
|
||||
* Which `package.json` files framework detection reads
|
||||
* (`src/resolution/frameworks/package-deps.ts`).
|
||||
*
|
||||
* The index lists source files, never manifests, so the candidates are
|
||||
* directories probed on disk. An ASP.NET solution keeps its single-page app
|
||||
* three levels down: jasontaylordev/CleanArchitecture's Angular app is
|
||||
* `src/Web/ClientApp/` and its React twin `src/Web/ClientApp-React/`.
|
||||
* prometheus keeps its React apps in `web/ui/mantine-ui/` and
|
||||
* `web/ui/react-app/`, under a workspace root at `web/ui/` that declares only
|
||||
* tooling. Read two levels deep, none of those manifests was read, so Angular
|
||||
* Router, the Angular template pass and React Router's navigation never ran,
|
||||
* and every template-bound handler was listed as dead code.
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { CodeGraph } from '../src';
|
||||
import { buildDeadCodeReport } from '../src/graph/dead-code';
|
||||
import { declaredDependencies, dependsOn } from '../src/resolution/frameworks/package-deps';
|
||||
import type { ResolutionContext } from '../src/resolution/types';
|
||||
|
||||
/** A context over an indexed file list and the manifests on disk, recording every probe. */
|
||||
function tree(files: string[], manifests: Record<string, Record<string, Record<string, string>>>) {
|
||||
const probes: string[] = [];
|
||||
const context = {
|
||||
getAllFiles: () => files,
|
||||
fileExists: (p: string) => {
|
||||
probes.push(p);
|
||||
return p in manifests;
|
||||
},
|
||||
readFile: (p: string) => (p in manifests ? JSON.stringify(manifests[p]) : null),
|
||||
} as unknown as ResolutionContext;
|
||||
return { context, probes };
|
||||
}
|
||||
|
||||
describe('declaredDependencies: manifests below the first two levels', () => {
|
||||
it.each([
|
||||
['src/Web/ClientApp/', 'an ASP.NET solution'],
|
||||
['src/Presentation/Web/ClientApp/', 'a solution folder deeper still'],
|
||||
])('reads %s, where %s keeps its app', (app) => {
|
||||
const { context } = tree(
|
||||
['src/Web/Program.cs', `${app}karma.conf.js`, `${app}src/main.ts`, `${app}src/app/app.module.ts`],
|
||||
{ [`${app}package.json`]: { dependencies: { '@angular/core': '^21.1.5', '@angular/router': '^21.1.5' } } }
|
||||
);
|
||||
expect(dependsOn(context, '@angular/router')).toBe(true);
|
||||
});
|
||||
|
||||
it('reads a workspace root two levels down and the apps a level below it', () => {
|
||||
const { context } = tree(
|
||||
['cmd/prometheus/main.go', 'web/ui/mantine-ui/src/main.tsx', 'web/ui/react-app/src/App.tsx', 'web/ui/module/lezer-promql/src/index.ts'],
|
||||
{
|
||||
'web/ui/package.json': { devDependencies: { typescript: '*', vite: '*' } },
|
||||
'web/ui/mantine-ui/package.json': { dependencies: { react: '*', 'react-router-dom': '*' } },
|
||||
'web/ui/react-app/package.json': { dependencies: { react: '*', 'react-router': '*' } },
|
||||
}
|
||||
);
|
||||
const names = declaredDependencies(context);
|
||||
expect([...names].sort()).toEqual(['react', 'react-router', 'react-router-dom', 'typescript', 'vite']);
|
||||
});
|
||||
|
||||
it('never reads a manifest inside node_modules', () => {
|
||||
const { context, probes } = tree(
|
||||
['tools/site/node_modules/framework/dist/index.js', 'tools/site/src/index.ts'],
|
||||
{ 'tools/site/node_modules/framework/package.json': { dependencies: { next: '*' } } }
|
||||
);
|
||||
expect(dependsOn(context, 'next')).toBe(false);
|
||||
expect(probes.filter((p) => p.includes('node_modules'))).toEqual([]);
|
||||
});
|
||||
|
||||
it('probes only above JS/TS code, and a bounded number of directories however many hold it', () => {
|
||||
const files = ['src/Api/Tools/Generator.cs'];
|
||||
for (let i = 0; i < 3000; i++) files.push(`src/app/feature${i}/parts/widget.component.ts`);
|
||||
const { context, probes } = tree(files, { 'src/Api/Tools/package.json': { dependencies: { express: '*' } } });
|
||||
expect(dependsOn(context, 'express')).toBe(false);
|
||||
// `src/`, `src/Api/`, `src/app/` from the first two levels; the rest below them.
|
||||
expect(probes.length).toBeLessThanOrEqual(3 + 192);
|
||||
});
|
||||
|
||||
it('reads the same manifests whatever order the files are listed in', () => {
|
||||
const files: string[] = [];
|
||||
const manifests: Record<string, Record<string, Record<string, string>>> = {};
|
||||
for (let i = 0; i < 40; i++) {
|
||||
files.push(`clients/web/app${i}/src/index.ts`);
|
||||
manifests[`clients/web/app${i}/package.json`] = { dependencies: { [`dep-${i}`]: '*' } };
|
||||
}
|
||||
const scanned = declaredDependencies(tree(files, manifests).context);
|
||||
const reversed = declaredDependencies(tree([...files].reverse(), manifests).context);
|
||||
expect([...reversed].sort()).toEqual([...scanned].sort());
|
||||
// A sample, as above two levels: the root and 24 more.
|
||||
expect(scanned.size).toBe(24);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// End to end: CleanArchitecture's shape
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const ngComponent = (name: string, selector: string, templateUrl: string, body = '', imports = '') => `import { Component } from '@angular/core';
|
||||
${imports}
|
||||
@Component({
|
||||
standalone: false,
|
||||
selector: '${selector}',
|
||||
templateUrl: '${templateUrl}'
|
||||
})
|
||||
export class ${name} {
|
||||
${body}
|
||||
}
|
||||
`;
|
||||
|
||||
describe('an ASP.NET solution with its single-page apps three levels down, indexed', () => {
|
||||
let root: string;
|
||||
let cg: CodeGraph;
|
||||
|
||||
beforeAll(async () => {
|
||||
root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-deep-manifest-'));
|
||||
const files: Record<string, string> = {
|
||||
'src/Web/Program.cs': 'var app = WebApplication.CreateBuilder(args).Build();\napp.Run();\n',
|
||||
'src/Web/ClientApp/package.json': JSON.stringify({ dependencies: { '@angular/core': '^21.1.5', '@angular/router': '^21.1.5' } }),
|
||||
'src/Web/ClientApp/angular.json': '{}',
|
||||
'src/Web/ClientApp/src/app/app.module.ts': `import { NgModule } from '@angular/core';
|
||||
import { RouterModule } from '@angular/router';
|
||||
import { HomeComponent } from './home/home.component';
|
||||
import { CounterComponent } from './counter/counter.component';
|
||||
import { LoginComponent } from './login/login.component';
|
||||
|
||||
@NgModule({
|
||||
imports: [
|
||||
RouterModule.forRoot([
|
||||
{ path: '', component: HomeComponent, pathMatch: 'full' },
|
||||
{ path: 'counter', component: CounterComponent },
|
||||
{ path: 'login', component: LoginComponent }
|
||||
])
|
||||
]
|
||||
})
|
||||
export class AppModule {}
|
||||
`,
|
||||
'src/Web/ClientApp/src/app/home/home.component.ts': ngComponent('HomeComponent', 'app-home', './home.component.html'),
|
||||
'src/Web/ClientApp/src/app/home/home.component.html': '<h1>Hello, world!</h1>\n',
|
||||
'src/Web/ClientApp/src/app/login/login.component.ts': ngComponent('LoginComponent', 'app-login', './login.component.html'),
|
||||
'src/Web/ClientApp/src/app/login/login.component.html': '<h1>Log in</h1>\n',
|
||||
'src/Web/ClientApp/src/app/counter/counter.component.ts': ngComponent(
|
||||
'CounterComponent',
|
||||
'app-counter-component',
|
||||
'./counter.component.html',
|
||||
' public currentCount = 0;\n\n public incrementCounter() {\n this.currentCount++;\n }'
|
||||
),
|
||||
'src/Web/ClientApp/src/app/counter/counter.component.html': '<p>Current count: {{ currentCount }}</p>\n<button (click)="incrementCounter()">Increment</button>\n',
|
||||
'src/Web/ClientApp/src/app/nav-menu/nav-menu.component.ts': ngComponent(
|
||||
'NavMenuComponent',
|
||||
'app-nav-menu',
|
||||
'./nav-menu.component.html',
|
||||
" constructor(private router: Router) {}\n\n logout(): void {\n this.router.navigate(['/login']);\n }",
|
||||
"import { Router } from '@angular/router';"
|
||||
),
|
||||
'src/Web/ClientApp/src/app/nav-menu/nav-menu.component.html': `<a [routerLink]="['/counter']">Counter</a>
|
||||
<a href="#" (click)="logout()">Log out</a>
|
||||
`,
|
||||
'src/Web/ClientApp-React/package.json': JSON.stringify({ dependencies: { react: '^19.1.0', 'react-dom': '^19.1.0', 'react-router-dom': '^7.6.1' } }),
|
||||
'src/Web/ClientApp-React/src/App.jsx': `import { Routes, Route } from 'react-router-dom';
|
||||
import { Home } from './components/Home';
|
||||
import { Counter } from './components/Counter';
|
||||
import { LoginPage } from './components/LoginPage';
|
||||
|
||||
export default function App() {
|
||||
return (
|
||||
<Routes>
|
||||
<Route path="/" element={<Home />} />
|
||||
<Route path="/counter" element={<Counter />} />
|
||||
<Route path="/login" element={<LoginPage />} />
|
||||
</Routes>
|
||||
);
|
||||
}
|
||||
`,
|
||||
'src/Web/ClientApp-React/src/components/Home.jsx': 'export function Home() {\n return <h1>Hello, world!</h1>;\n}\n',
|
||||
'src/Web/ClientApp-React/src/components/Counter.jsx': 'export function Counter() {\n return <h1>Counter</h1>;\n}\n',
|
||||
'src/Web/ClientApp-React/src/components/LoginPage.jsx': 'export function LoginPage() {\n return <h1>Log in</h1>;\n}\n',
|
||||
'src/Web/ClientApp-React/src/components/NavMenu.jsx': `import { Link, useNavigate } from 'react-router-dom';
|
||||
|
||||
export function NavMenu() {
|
||||
const navigate = useNavigate();
|
||||
const handleLogout = (e) => {
|
||||
e.preventDefault();
|
||||
navigate('/login');
|
||||
};
|
||||
return (
|
||||
<nav>
|
||||
<Link to="/counter">Counter</Link>
|
||||
<a href="#" onClick={handleLogout}>Log out</a>
|
||||
</nav>
|
||||
);
|
||||
}
|
||||
`,
|
||||
};
|
||||
for (const [rel, content] of Object.entries(files)) {
|
||||
fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true });
|
||||
fs.writeFileSync(path.join(root, rel), content);
|
||||
}
|
||||
cg = await CodeGraph.init(root, { index: true });
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
cg?.close();
|
||||
fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
const routesUnder = (dir: string) => cg.getNodesByKind('route').filter((r) => r.filePath.startsWith(dir));
|
||||
const navigations = (dir: string) =>
|
||||
routesUnder(dir)
|
||||
.flatMap((r) =>
|
||||
cg.getIncomingEdgesTo([r.id], ['navigates']).map((e) => `${cg.getNode(e.source)?.name} -> ${r.name} (${(e.metadata as Record<string, unknown>).navMethod})`)
|
||||
)
|
||||
.sort();
|
||||
|
||||
it('detects the frameworks the nested manifests declare', () => {
|
||||
expect(cg.getDetectedFrameworks()).toEqual(expect.arrayContaining(['angular-router', 'react-router']));
|
||||
});
|
||||
|
||||
it("reads the Angular app's routes, bound to their components", () => {
|
||||
const routes = routesUnder('src/Web/ClientApp/')
|
||||
.map((r) => `${r.name} -> ${cg.getOutgoingEdgesFrom([r.id], ['references']).map((e) => cg.getNode(e.target)?.name).join(', ')}`)
|
||||
.sort();
|
||||
expect(routes).toEqual(['/ -> HomeComponent', '/counter -> CounterComponent', '/login -> LoginComponent']);
|
||||
});
|
||||
|
||||
it("draws the Angular app's navigation from routerLink and router.navigate", () => {
|
||||
expect(navigations('src/Web/ClientApp/')).toEqual(['NavMenuComponent -> /counter (routerLink)', 'logout -> /login (navigate)']);
|
||||
});
|
||||
|
||||
it("links a template's event binding to its handler, so the handler is not dead code", () => {
|
||||
const counter = cg.getNodesByKind('class').find((c) => c.name === 'CounterComponent')!;
|
||||
const handlers = cg
|
||||
.getOutgoingEdgesFrom([counter.id], ['calls'])
|
||||
.filter((e) => (e.metadata as Record<string, unknown> | undefined)?.synthesizedBy === 'angular-event')
|
||||
.map((e) => cg.getNode(e.target)?.name);
|
||||
expect(handlers).toEqual(['incrementCounter']);
|
||||
const dead = buildDeadCodeReport(cg, { limit: 1000 }).entries.map((e) => e.node.name);
|
||||
expect(dead).not.toContain('incrementCounter');
|
||||
expect(dead).not.toContain('logout');
|
||||
});
|
||||
|
||||
it("resolves the React twin's navigate() as well as its <Link to>", () => {
|
||||
expect(navigations('src/Web/ClientApp-React/')).toEqual(['NavMenu -> /counter (link)', 'handleLogout -> /login (navigate)']);
|
||||
});
|
||||
});
|
||||
@@ -231,6 +231,16 @@ Each of these cost real debugging time; they are not hypothetical.
|
||||
per file-count for exactly this reason — an earlier version cached the empty
|
||||
pre-index answer and every framework whose dependency lived one directory
|
||||
down stayed undetected.
|
||||
12. **An app's manifest can sit three or more levels down.** An ASP.NET
|
||||
solution keeps its single-page app in `src/Web/ClientApp/`, and prometheus
|
||||
keeps its React apps in `web/ui/mantine-ui/` under a workspace root at
|
||||
`web/ui/` that declares only tooling. `declaredDependencies` reads the root
|
||||
and the first two levels, then gives the slots left to the directories
|
||||
above JS/TS code, shallowest first. Read only two levels deep, Angular
|
||||
Router never ran on jasontaylordev/CleanArchitecture, and every
|
||||
template-bound handler there was listed as dead code. A new detector that
|
||||
gates on a dependency should ask `dependsOn`, not read a `package.json`
|
||||
itself, so it sees the same manifests every other detector does.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -1,10 +1,12 @@
|
||||
/**
|
||||
* The dependencies a project declares — in its root `package.json` and in
|
||||
* the ones one or two directories down (`apps/web/package.json`,
|
||||
* `frontend/package.json`, `packages/api/package.json`). A framework detector
|
||||
* that reads only the root misses every monorepo: proshop keeps `react` in
|
||||
* `frontend/`, a Turborepo keeps `next` in `apps/web/`, and the resolver for
|
||||
* that framework then never runs, so its routes never exist.
|
||||
* The dependencies a project declares — in its root `package.json`, in the
|
||||
* ones one or two directories down (`apps/web/package.json`,
|
||||
* `frontend/package.json`, `packages/api/package.json`), and in the ones
|
||||
* deeper still above JS/TS code (`src/Web/ClientApp/package.json`). A
|
||||
* framework detector that reads only the root misses every monorepo: proshop
|
||||
* keeps `react` in `frontend/`, a Turborepo keeps `next` in `apps/web/`, an
|
||||
* ASP.NET solution keeps its Angular app in `src/Web/ClientApp/`, and the
|
||||
* resolver for that framework then never runs, so its routes never exist.
|
||||
*/
|
||||
|
||||
import type { ResolutionContext } from '../types';
|
||||
@@ -12,6 +14,12 @@ import type { ResolutionContext } from '../types';
|
||||
/** Nested manifests read per project, at most — a monorepo with hundreds of packages is sampled, not scanned. */
|
||||
const MAX_MANIFESTS = 24;
|
||||
|
||||
/** Directories probed for a manifest below the first two levels, at most. */
|
||||
const MAX_DEEP_PROBES = MAX_MANIFESTS * 8;
|
||||
|
||||
/** A file a JavaScript framework's app is written in. Its app's manifest is in a directory above it. */
|
||||
const SCRIPT_FILE = /\.(?:[cm]?[jt]sx?|vue|svelte|astro)$/;
|
||||
|
||||
/**
|
||||
* Cached per context, keyed by how many files are indexed.
|
||||
*
|
||||
@@ -26,7 +34,7 @@ const MAX_MANIFESTS = 24;
|
||||
*/
|
||||
const cache = new WeakMap<ResolutionContext, { files: number; names: Set<string> }>();
|
||||
|
||||
/** Every dependency name declared at the root or up to two directories down, de-duplicated. */
|
||||
/** Every dependency name declared in the manifests read (see the file comment), de-duplicated. */
|
||||
export function declaredDependencies(context: ResolutionContext): Set<string> {
|
||||
const files = context.getAllFiles();
|
||||
const cached = cache.get(context);
|
||||
@@ -46,6 +54,16 @@ export function declaredDependencies(context: ResolutionContext): Set<string> {
|
||||
if (manifests.length > MAX_MANIFESTS) break;
|
||||
if (!dir.includes('node_modules') && context.fileExists(dir + 'package.json')) manifests.push(dir + 'package.json');
|
||||
}
|
||||
// An app can sit deeper: an ASP.NET solution's single-page app in
|
||||
// `src/Web/ClientApp/`, prometheus's React apps in `web/ui/mantine-ui/`
|
||||
// under a workspace root at `web/ui/`. The slots left go to the directories
|
||||
// above JS/TS code, shallowest first.
|
||||
if (manifests.length <= MAX_MANIFESTS) {
|
||||
for (const dir of deepScriptDirectories(files)) {
|
||||
if (manifests.length > MAX_MANIFESTS) break;
|
||||
if (context.fileExists(dir + 'package.json')) manifests.push(dir + 'package.json');
|
||||
}
|
||||
}
|
||||
for (const manifest of manifests) {
|
||||
const content = context.readFile(manifest);
|
||||
if (!content) continue;
|
||||
@@ -62,6 +80,34 @@ export function declaredDependencies(context: ResolutionContext): Set<string> {
|
||||
return names;
|
||||
}
|
||||
|
||||
/**
|
||||
* The directories three or more levels down that hold indexed JS/TS code or a
|
||||
* directory that does, never inside `node_modules`: shallowest first, at most
|
||||
* {@link MAX_DEEP_PROBES} of them. Sorted within a level, so the sample does
|
||||
* not depend on the order the files come in. Extraction lists them as git or a
|
||||
* directory walk returns them, the resolver in path order, and each decides
|
||||
* from its own list which frameworks run.
|
||||
*/
|
||||
function deepScriptDirectories(files: readonly string[]): string[] {
|
||||
// levels[n]: the directories n levels down (`a/b/c/` is 3).
|
||||
const levels: Set<string>[] = [];
|
||||
for (const file of files) {
|
||||
if (!SCRIPT_FILE.test(file) || file.includes('node_modules')) continue;
|
||||
let depth = 0;
|
||||
for (let slash = file.indexOf('/'); slash >= 0; slash = file.indexOf('/', slash + 1)) {
|
||||
if (++depth >= 3) (levels[depth] ??= new Set()).add(file.slice(0, slash + 1));
|
||||
}
|
||||
}
|
||||
const dirs: string[] = [];
|
||||
for (let depth = 3; depth < levels.length; depth++) {
|
||||
for (const dir of [...(levels[depth] ?? [])].sort()) {
|
||||
if (dirs.length === MAX_DEEP_PROBES) return dirs;
|
||||
dirs.push(dir);
|
||||
}
|
||||
}
|
||||
return dirs;
|
||||
}
|
||||
|
||||
/** True when any of `deps` is declared anywhere the project's manifests are read. */
|
||||
export function dependsOn(context: ResolutionContext, ...deps: string[]): boolean {
|
||||
const names = declaredDependencies(context);
|
||||
|
||||
Reference in New Issue
Block a user