From a572f40b3e8dcf885ec82ec6fe1ce0a38a3d61b5 Mon Sep 17 00:00:00 2001 From: Kam Date: Fri, 9 Oct 2026 22:18:19 +0300 Subject: [PATCH 1/2] fix: correct 35 P3 collector, scanner, lint and agent tool bugs Source scanners: named child route arrays nest under their parent route, shorthand and quoted route keys are read, signalStore type arguments no longer split on commas, pipe uses in string literals are ignored and animation bindings are read, decorator inputs with a closing parenthesis in their options are kept, and feature folders named build, coverage or tmp below a source root are scanned again. Lints and agent tools: duplicate-path, redirect-cycle, explain-render-mode, @for block matching, nested @for findings, api-not-found, restart-needed, form-diff, form-history, no-focus-on-invalid-submit and the hidden input label rule no longer report things that are not wrong, and fill with submit reports a blocked or skipped submit as a failure. Page side: router probe timeout aborts the navigation and marks it as a probe, CD and NativeScript keepalives follow the tick length, a failing NativeScript push no longer stops the other collectors, Angular Native signal pushes roll back and carry resource changes, store references are rebuilt on every pass, and the docked popup no longer draws a dead resize grip. The tools page now states the 51 tools the server registers, with a test that keeps it in step. --- apps/docs/src/content/agents/tools.md | 2 +- .../src/__tests__/analog-tools-gaps.test.ts | 100 ++++++++++++++++ .../__tests__/cd-overlay-keepalive.test.ts | 49 ++++++++ .../src/__tests__/docs-tool-count.test.ts | 34 ++++++ .../forms-actions-fill-submit.test.ts | 99 ++++++++++++++++ .../__tests__/forms-explain-unmatched.test.ts | 46 ++++++++ .../__tests__/forms-instrument-frames.test.ts | 25 ++++ .../__tests__/forms-lint-submit-focus.test.ts | 65 ++++++++++ .../src/__tests__/forms-source-gaps.test.ts | 62 ++++++++++ .../src/__tests__/host-tree-body-root.test.ts | 20 ++++ .../src/__tests__/ngrx-collector.test.ts | 7 ++ .../__tests__/overlay-angular-native.test.ts | 29 +++++ .../overlay-nativescript-report.test.ts | 111 ++++++++++++++++++ .../src/__tests__/pipes-runtime.test.ts | 19 +++ packages/devtools/src/__tests__/popup.test.ts | 12 ++ .../router-actions-probe-timeout.test.ts | 70 +++++++++++ .../router-config-tools-gaps.test.ts | 58 +++++++++ .../src/__tests__/router-gaps.test.ts | 55 +++++++++ packages/devtools/src/cd-overlay.ts | 7 +- packages/devtools/src/forms-actions.ts | 8 +- packages/devtools/src/forms-dom.ts | 5 +- packages/devtools/src/forms-instrument.ts | 2 +- packages/devtools/src/host-tree.ts | 1 + packages/devtools/src/ngrx-collector.ts | 1 + .../devtools/src/overlay-angular-native.ts | 35 ++++-- packages/devtools/src/overlay-nativescript.ts | 26 ++-- packages/devtools/src/overlay.ts | 4 +- packages/devtools/src/pipes-runtime.ts | 13 +- packages/devtools/src/popup.ts | 4 +- packages/devtools/src/router-actions.ts | 2 + packages/devtools/src/router-config.ts | 6 +- packages/devtools/src/router-setup.ts | 4 +- .../get-components-decorators.test.ts | 29 +++++ .../__tests__/get-ngrx-store-generics.test.ts | 30 +++++ .../__tests__/get-pipes-expressions.test.ts | 21 ++++ .../src/rpc/__tests__/get-routes-gaps.test.ts | 35 ++++++ .../src/rpc/__tests__/pipe-explain.test.ts | 22 ++++ .../src/rpc/__tests__/pipe-lint-for.test.ts | 32 +++++ .../source-scan-feature-folders.test.ts | 27 +++++ packages/devtools/src/rpc/analog-tools.ts | 48 ++++++-- packages/devtools/src/rpc/forms-explain.ts | 15 +++ packages/devtools/src/rpc/forms-lint.ts | 7 +- packages/devtools/src/rpc/forms-source.ts | 34 ++++-- packages/devtools/src/rpc/get-components.ts | 27 +++-- packages/devtools/src/rpc/get-ngrx-store.ts | 16 ++- packages/devtools/src/rpc/get-pipes.ts | 5 +- packages/devtools/src/rpc/get-routes.ts | 26 +++- packages/devtools/src/rpc/pipe-explain.ts | 10 +- packages/devtools/src/rpc/pipe-lint.ts | 14 ++- .../devtools/src/rpc/router-config-tools.ts | 17 ++- packages/devtools/src/rpc/router-loops.ts | 7 +- packages/devtools/src/rpc/source-scan.ts | 13 +- 52 files changed, 1326 insertions(+), 90 deletions(-) create mode 100644 packages/devtools/src/__tests__/analog-tools-gaps.test.ts create mode 100644 packages/devtools/src/__tests__/cd-overlay-keepalive.test.ts create mode 100644 packages/devtools/src/__tests__/docs-tool-count.test.ts create mode 100644 packages/devtools/src/__tests__/forms-actions-fill-submit.test.ts create mode 100644 packages/devtools/src/__tests__/forms-explain-unmatched.test.ts create mode 100644 packages/devtools/src/__tests__/forms-instrument-frames.test.ts create mode 100644 packages/devtools/src/__tests__/forms-lint-submit-focus.test.ts create mode 100644 packages/devtools/src/__tests__/forms-source-gaps.test.ts create mode 100644 packages/devtools/src/__tests__/host-tree-body-root.test.ts create mode 100644 packages/devtools/src/__tests__/overlay-nativescript-report.test.ts create mode 100644 packages/devtools/src/__tests__/router-actions-probe-timeout.test.ts create mode 100644 packages/devtools/src/__tests__/router-config-tools-gaps.test.ts create mode 100644 packages/devtools/src/__tests__/router-gaps.test.ts create mode 100644 packages/devtools/src/rpc/__tests__/get-components-decorators.test.ts create mode 100644 packages/devtools/src/rpc/__tests__/get-ngrx-store-generics.test.ts create mode 100644 packages/devtools/src/rpc/__tests__/get-pipes-expressions.test.ts create mode 100644 packages/devtools/src/rpc/__tests__/get-routes-gaps.test.ts create mode 100644 packages/devtools/src/rpc/__tests__/pipe-lint-for.test.ts create mode 100644 packages/devtools/src/rpc/__tests__/source-scan-feature-folders.test.ts diff --git a/apps/docs/src/content/agents/tools.md b/apps/docs/src/content/agents/tools.md index 23e4572a..f3129084 100644 --- a/apps/docs/src/content/agents/tools.md +++ b/apps/docs/src/content/agents/tools.md @@ -4,7 +4,7 @@ description: Every agent tool the devtools expose, grouped by inspector, with wh --- - Forty-eight tools, grouped by inspector. Each one answers a question you would otherwise answer by clicking through the panel. + Fifty-one tools, grouped by inspector. Each one answers a question you would otherwise answer by clicking through the panel. # Tools diff --git a/packages/devtools/src/__tests__/analog-tools-gaps.test.ts b/packages/devtools/src/__tests__/analog-tools-gaps.test.ts new file mode 100644 index 00000000..a68619e8 --- /dev/null +++ b/packages/devtools/src/__tests__/analog-tools-gaps.test.ts @@ -0,0 +1,100 @@ +import { describe, expect, it } from 'vitest'; +import type { AnalogCall } from '../analog-server-log.ts'; +import type { AnalogRuntimeReport } from '../analog-runtime.ts'; +import { scanAnalog } from '../rpc/analog-scan.ts'; +import { analogLint, restartNeeded } from '../rpc/analog-tools.ts'; +import { BASE_FILES, makeProject } from './analog-fixture.ts'; + +const call = (extra: Partial): AnalogCall => ({ + id: 1, + at: 1, + kind: 'api', + method: 'GET', + url: '/api/v1/hello', + route: '/api/v1/hello', + status: 404, + ms: 1, + from: 'browser', + ...extra, +}); + +const report = (configPaths: string[]): AnalogRuntimeReport => ({ + pageId: 'p', + url: '/', + analog: true, + chain: [], + serverContext: 'client', + hydrated: 0, + transferState: false, + hydrationErrors: [], + configPaths, +}); + +const lintCalls = (calls: AnalogCall[]) => + analogLint(scanAnalog(makeProject(BASE_FILES)), { + pages: [], + calls, + duplicates: [], + reportedAt: 1, + }).filter((f) => f.rule === 'api-not-found'); + +describe('api-not-found lint', () => { + it('does not blame the file layout for a 404 that an existing handler returned', () => { + const calls = [ + call({ id: 1, method: 'GET', url: '/api/v1/hello', route: '/api/v1/hello' }), + call({ id: 2, method: 'GET', url: '/api/v1/hello', route: '/api/v1/hello' }), + ]; + expect(lintCalls(calls)).toEqual([]); + }); + + it('matches a handler with a parameter segment', () => { + const calls = [ + call({ method: 'DELETE', url: '/api/v1/products/9', route: '/api/v1/products/9' }), + ]; + expect(lintCalls(calls)).toEqual([]); + }); + + it('reports a missing route once however often it was called', () => { + const calls = [ + call({ id: 1, url: '/api/v1/nope', route: '/api/v1/nope' }), + call({ id: 2, url: '/api/v1/nope', route: '/api/v1/nope' }), + call({ id: 3, url: '/api/v1/nope', route: '/api/v1/nope' }), + ]; + expect(lintCalls(calls)).toHaveLength(1); + }); + + it('reports a 405 when the path exists only for another method', () => { + const calls = [ + call({ method: 'POST', status: 405, url: '/api/v1/products', route: '/api/v1/products' }), + ]; + expect(lintCalls(calls)).toHaveLength(1); + }); +}); + +describe('restartNeeded', () => { + const project = scanAnalog(makeProject(BASE_FILES)); + const known = [ + '/', + '/products', + '/products/:id', + '/login', + '/pricing', + '/about', + '/dashboard', + '/hello', + ]; + + it('notices a page added under a path the router already knows', () => { + const missing = restartNeeded(project, report(known.filter((p) => p !== '/products/:id'))); + expect(missing).toContain('/products/:id'); + }); + + it('reports nothing when the router knows every page', () => { + expect(restartNeeded(project, report(known))).toEqual([]); + }); + + it('does not guess when the router path list hit its cap', () => { + const capped = Array.from({ length: 500 }, (_, i) => `/p${i}`); + expect(restartNeeded(project, report(capped))).toEqual([]); + }); +}); diff --git a/packages/devtools/src/__tests__/cd-overlay-keepalive.test.ts b/packages/devtools/src/__tests__/cd-overlay-keepalive.test.ts new file mode 100644 index 00000000..9e2e688d --- /dev/null +++ b/packages/devtools/src/__tests__/cd-overlay-keepalive.test.ts @@ -0,0 +1,49 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { attachChangeDetection } from '../cd-overlay.ts'; + +describe('change detection keepalive', () => { + beforeEach(() => { + vi.useFakeTimers(); + document.body.innerHTML = ''; + }); + + afterEach(() => { + vi.useRealTimers(); + document.body.innerHTML = ''; + }); + + it('pings before the page expires when a tick lands just short of the keepalive', async () => { + const ng = { ɵsetProfiler: () => () => {}, getHostElement: () => null }; + const sent: { name: string; at: number }[] = []; + const handlers = new Map unknown>(); + const my = { + rpc: { + call: async (name: string) => { + sent.push({ name, at: Date.now() }); + return undefined; + }, + register: (def: { name: string; handler: (...args: unknown[]) => unknown }) => + handlers.set(def.name, def.handler), + }, + }; + const tick = 8000; + const cd = attachChangeDetection( + my, + 'p1', + () => ng, + 50, + () => tick, + ); + handlers.get('change-detection-record')!({ pageId: 'p1', on: true }); + await vi.advanceTimersByTimeAsync(0); + const start = Date.now(); + for (let elapsed = 0; elapsed < 30_000; elapsed += tick - 1) { + vi.setSystemTime(start + elapsed); + await cd.push(); + } + const times = sent.map((s) => s.at); + const gaps = times.slice(1).map((at, i) => at - times[i]); + expect(gaps.every((gap) => gap <= 15_000)).toBe(true); + }); +}); diff --git a/packages/devtools/src/__tests__/docs-tool-count.test.ts b/packages/devtools/src/__tests__/docs-tool-count.test.ts new file mode 100644 index 00000000..ce064932 --- /dev/null +++ b/packages/devtools/src/__tests__/docs-tool-count.test.ts @@ -0,0 +1,34 @@ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { createHostContext } from 'devframe/node'; +import { expect, it } from 'vitest'; +import pangular from '../devframe.ts'; + +const WORDS = [ + 'Forty-eight', + 'Forty-nine', + 'Fifty', + 'Fifty-one', + 'Fifty-two', + 'Fifty-three', + 'Fifty-four', + 'Fifty-five', +]; + +it('states on the tools page how many tools the server registers', async () => { + const host = { + mountStatic: () => {}, + resolveOrigin: () => 'http://localhost', + getStorageDir: () => '', + }; + const ctx = await createHostContext({ cwd: process.cwd(), mode: 'dev', host: host as never }); + await pangular.setup(ctx as never); + const registered = ctx.agent.list().tools.length; + const page = readFileSync( + join(__dirname, '../../../../apps/docs/src/content/agents/tools.md'), + 'utf-8', + ); + const stated = /^\s*([A-Z][a-z]+(?:-[a-z]+)?) tools, grouped by inspector/m.exec(page)?.[1]; + expect(stated).toBeDefined(); + expect(WORDS.indexOf(stated!) + 48).toBe(registered); +}); diff --git a/packages/devtools/src/__tests__/forms-actions-fill-submit.test.ts b/packages/devtools/src/__tests__/forms-actions-fill-submit.test.ts new file mode 100644 index 00000000..14f57a95 --- /dev/null +++ b/packages/devtools/src/__tests__/forms-actions-fill-submit.test.ts @@ -0,0 +1,99 @@ +// @vitest-environment jsdom +import '@angular/compiler'; +import { Component } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; +import { FormControl, FormGroup, ReactiveFormsModule } from '@angular/forms'; +import { BrowserTestingModule, platformBrowserTesting } from '@angular/platform-browser/testing'; +import { afterEach, describe, expect, it } from 'vitest'; +import { findForms } from '../forms.ts'; +import { runFormAction, type ActionContext } from '../forms-actions.ts'; + +try { + TestBed.initTestEnvironment(BrowserTestingModule, platformBrowserTesting()); +} catch { + // already initialized in this worker +} + +afterEach(() => TestBed.resetTestingModule()); + +function contextFor(host: HTMLElement): ActionContext { + const ng = (globalThis as any).ng; + const all = () => host.querySelectorAll('*'); + const found = findForms(ng, all()); + const forms = new Map(found.forms.map((f, i) => [`form-${i + 1}`, f])); + return { ng, forms, elements: found.elements, all }; +} + +class Notes { + submits = 0; + form = new FormGroup({ + note: new FormControl(''), + title: new FormControl(''), + password: new FormControl(''), + }); +} +Component({ + selector: 'notes-form', + imports: [ReactiveFormsModule], + template: ` +
+ + + + +
+ `, +})(Notes); + +async function render() { + const fixture = TestBed.createComponent(Notes); + fixture.detectChanges(); + await fixture.whenStable(); + fixture.detectChanges(); + return fixture; +} + +describe('fill with submit', () => { + it('reports a blocked submit as a failure and does not submit', async () => { + const fixture = await render(); + const ctx = contextFor(fixture.nativeElement); + const result = await runFormAction(ctx, { + action: 'fill', + formId: 'form-1', + values: { note: 'x' }, + submit: true, + confirm: true, + }); + expect(result.ok).toBe(false); + expect(result.message).toContain('would block this submit'); + expect(fixture.componentInstance.submits).toBe(0); + }); + + it('does not submit when every field was skipped', async () => { + const fixture = await render(); + const ctx = contextFor(fixture.nativeElement); + const result = await runFormAction(ctx, { + action: 'fill', + formId: 'form-1', + values: { password: 'secret' }, + submit: true, + confirm: true, + }); + expect(result.ok).toBe(false); + expect(fixture.componentInstance.submits).toBe(0); + }); + + it('submits when the fields are written and the form is valid', async () => { + const fixture = await render(); + const ctx = contextFor(fixture.nativeElement); + const result = await runFormAction(ctx, { + action: 'fill', + formId: 'form-1', + values: { note: 'x', title: 'y' }, + submit: true, + confirm: true, + }); + expect(result.ok).toBe(true); + expect(fixture.componentInstance.submits).toBe(1); + }); +}); diff --git a/packages/devtools/src/__tests__/forms-explain-unmatched.test.ts b/packages/devtools/src/__tests__/forms-explain-unmatched.test.ts new file mode 100644 index 00000000..c295d488 --- /dev/null +++ b/packages/devtools/src/__tests__/forms-explain-unmatched.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from 'vitest'; +import type { CollectedForm, FormEvent } from '../forms.ts'; +import { formDiffText, formHistoryText } from '../rpc/forms-explain.ts'; + +const form = { + id: 'a@p1', + kind: 'reactive', + owner: 'A', + property: 'f', + label: 'A.f', + submitted: false, + root: { + key: 'f', + path: '', + type: 'group', + status: 'VALID', + touched: false, + dirty: false, + bound: false, + errors: [], + }, +} as unknown as CollectedForm; + +const events: FormEvent[] = [ + { formId: 'a@p1', path: 'x', type: 'value', detail: '"1"', prev: '""', timestamp: 1, seq: 1 }, +]; + +describe('form-diff and form-history with a form that matches nothing', () => { + const state = { forms: [form], events, reportedAt: 0 }; + + it('says no form matches instead of reporting no change', () => { + for (const text of [ + formDiffText(state, { form: 'zzz', since: 0 }), + formHistoryText(state, { form: 'zzz' }), + ]) { + expect(text).toContain('No form matches `zzz`'); + expect(text).toContain('a@p1'); + expect(text).not.toContain('Nothing changed'); + expect(text).not.toContain('No matching form events'); + } + }); + + it('still reports no change for a form that exists', () => { + expect(formDiffText(state, { form: 'a@p1', since: 1 })).toContain('Nothing changed'); + }); +}); diff --git a/packages/devtools/src/__tests__/forms-instrument-frames.test.ts b/packages/devtools/src/__tests__/forms-instrument-frames.test.ts new file mode 100644 index 00000000..98340539 --- /dev/null +++ b/packages/devtools/src/__tests__/forms-instrument-frames.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, it } from 'vitest'; +import { callerFrom } from '../forms-instrument.ts'; + +const framework = + ' at FormControl.setValue (http://localhost/node_modules/@angular/forms/forms.mjs:1:1)'; + +describe('callerFrom framework filter', () => { + it('keeps user frames whose file name only contains a framework word', () => { + for (const file of ['timezone-form.component.ts', 'rxjs-demo.ts', 'signup-forms-actions.ts']) { + const stack = `Error\n${framework}\n at TzComponent.save (http://localhost/src/app/${file}:10:5)`; + expect(callerFrom(stack, new Set(['TzComponent']))).toContain('TzComponent.save'); + } + }); + + it('still drops real framework frames', () => { + const stack = [ + 'Error', + framework, + ' at Subscriber.next (http://localhost/node_modules/rxjs/dist/esm/Subscriber.js:1:1)', + ' at ZoneDelegate.invoke (http://localhost/node_modules/zone.js/fesm2015/zone.js:1:1)', + ' at Object.onInvoke (http://localhost/polyfills/zone-evergreen.js:1:1)', + ].join('\n'); + expect(callerFrom(stack)).toBeUndefined(); + }); +}); diff --git a/packages/devtools/src/__tests__/forms-lint-submit-focus.test.ts b/packages/devtools/src/__tests__/forms-lint-submit-focus.test.ts new file mode 100644 index 00000000..b8829244 --- /dev/null +++ b/packages/devtools/src/__tests__/forms-lint-submit-focus.test.ts @@ -0,0 +1,65 @@ +// @vitest-environment jsdom +import { describe, expect, it } from 'vitest'; +import { domFacts } from '../forms-dom.ts'; +import type { CollectedForm, FormEvent } from '../forms.ts'; +import { lintForm } from '../rpc/forms-lint.ts'; + +const form = { + id: 'f@p', + kind: 'reactive', + owner: 'C', + label: 'C.form', + root: { + key: '', + path: '', + type: 'group', + status: 'VALID', + touched: false, + dirty: false, + bound: false, + errors: [], + children: [], + }, +} as unknown as CollectedForm; + +const submit = (detail: string, timestamp: number): FormEvent => ({ + formId: 'f@p', + path: '', + type: 'submit', + detail, + timestamp, +}); + +const rules = (events: FormEvent[]) => lintForm(form, events, 10_000).map((f) => f.rule); + +describe('no-focus-on-invalid-submit', () => { + it('flags an invalid submit that left focus on the button', () => { + expect(rules([submit('status INVALID at submit, focus on button[save]', 1)])).toContain( + 'no-focus-on-invalid-submit', + ); + }); + + it('is cleared by a later valid submit', () => { + const events = [ + submit('status INVALID at submit, focus on button', 1), + submit('status VALID at submit', 2), + ]; + expect(rules(events)).not.toContain('no-focus-on-invalid-submit'); + }); + + it('accepts focus moved to a custom control', () => { + expect( + rules([submit('status INVALID at submit, focus on mat-select[country]', 1)]), + ).not.toContain('no-focus-on-invalid-submit'); + }); +}); + +describe('hidden inputs', () => { + it('records no label fact for a hidden input', () => { + const el = document.createElement('input'); + el.type = 'hidden'; + expect(domFacts(el, {}).labelled).toBeUndefined(); + const visible = document.createElement('input'); + expect(domFacts(visible, {}).labelled).toBe(false); + }); +}); diff --git a/packages/devtools/src/__tests__/forms-source-gaps.test.ts b/packages/devtools/src/__tests__/forms-source-gaps.test.ts new file mode 100644 index 00000000..f4ad1890 --- /dev/null +++ b/packages/devtools/src/__tests__/forms-source-gaps.test.ts @@ -0,0 +1,62 @@ +import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { findFormSource, formSourceIn } from '../rpc/forms-source.ts'; + +describe('form source scan gaps', () => { + it('ignores parentheses inside string literals when finding the end of a schema', () => { + const page = `const S = schema((p) => { + validate(p.name, () => ({ kind: 'x', message: 'Use a name (first and last' })); + required(p.email); +}); + +export class Page { + f = form(this.model, S); +} + +function other() { + required(p.email); +} +`; + const found = formSourceIn(page, 'a.ts', 'Page', 'f', 'email'); + expect(found?.rules.map((r) => r.line)).toEqual([3]); + + const closing = page.replace('(first and last', 'a ) b'); + const early = formSourceIn(closing, 'a.ts', 'Page', 'f', 'email'); + expect(early?.rules.map((r) => r.line)).toEqual([3]); + }); + + it('takes a same-named schema from the file the owner imports it from', () => { + const cwd = mkdtempSync(join(tmpdir(), 'pangular-schema-')); + for (const dir of ['a', 'b']) { + mkdirSync(join(cwd, 'src', dir), { recursive: true }); + writeFileSync( + join(cwd, 'src', dir, 'schema.ts'), + `export const addressSchema = schema((p) => {\n required(p.city);\n});\n`, + ); + } + writeFileSync( + join(cwd, 'src', 'page.ts'), + `import { addressSchema } from './b/schema';\nexport class Page {\n f = form(this.model, addressSchema);\n}\n`, + ); + const found = findFormSource(cwd, 'Page', 'f', 'city'); + expect(found?.schemas?.map((s) => s.file.replaceAll('\\', '/'))).toEqual(['src/b/schema.ts']); + }); + + it('does not guess between same-named schemas it cannot tell apart', () => { + const cwd = mkdtempSync(join(tmpdir(), 'pangular-schema-')); + for (const dir of ['a', 'b']) { + mkdirSync(join(cwd, 'src', dir), { recursive: true }); + writeFileSync( + join(cwd, 'src', dir, 'schema.ts'), + `export const addressSchema = schema((p) => {\n required(p.city);\n});\n`, + ); + } + writeFileSync( + join(cwd, 'src', 'page.ts'), + `import { addressSchema } from '@app/schema';\nexport class Page {\n f = form(this.model, addressSchema);\n}\n`, + ); + expect(findFormSource(cwd, 'Page', 'f', 'city')?.schemas).toBeUndefined(); + }); +}); diff --git a/packages/devtools/src/__tests__/host-tree-body-root.test.ts b/packages/devtools/src/__tests__/host-tree-body-root.test.ts new file mode 100644 index 00000000..e2ad38be --- /dev/null +++ b/packages/devtools/src/__tests__/host-tree-body-root.test.ts @@ -0,0 +1,20 @@ +// @vitest-environment jsdom +import { describe, expect, it } from 'vitest'; +import { angularRoots } from '../host-tree.ts'; + +describe('angularRoots', () => { + it('returns only the body when the app is bootstrapped onto it', () => { + document.body.setAttribute('ng-version', '20.0.0'); + document.body.innerHTML = ''; + expect(angularRoots(document)).toEqual([document.body]); + document.body.removeAttribute('ng-version'); + }); + + it('still lists elements outside the Angular roots', () => { + document.body.innerHTML = '
'; + expect(angularRoots(document).map((el) => el.tagName.toLowerCase())).toEqual([ + 'app-root', + 'footer', + ]); + }); +}); diff --git a/packages/devtools/src/__tests__/ngrx-collector.test.ts b/packages/devtools/src/__tests__/ngrx-collector.test.ts index 78959364..8f7ef290 100644 --- a/packages/devtools/src/__tests__/ngrx-collector.test.ts +++ b/packages/devtools/src/__tests__/ngrx-collector.test.ts @@ -101,6 +101,13 @@ describe('ngrx collector', () => { expect(info.writable).toBe(true); }); + it('drops a component that no longer holds the store from its references', () => { + const { app, collector } = setup(); + expect(collector.collect().stores[0].references).toEqual(['App.store']); + delete (app as { store?: unknown }).store; + expect(collector.collect().stores[0].references).toEqual([]); + }); + it('unwraps a store that is no longer found and wraps it once when it comes back', () => { const { store, app, rootEnv, collector } = setup(); const original = store.setQuery; diff --git a/packages/devtools/src/__tests__/overlay-angular-native.test.ts b/packages/devtools/src/__tests__/overlay-angular-native.test.ts index fa1de4b9..76ce1682 100644 --- a/packages/devtools/src/__tests__/overlay-angular-native.test.ts +++ b/packages/devtools/src/__tests__/overlay-angular-native.test.ts @@ -441,6 +441,35 @@ describe('initAngularNativeOverlay', () => { warn.mockRestore(); }); + it('sends the signal graph again on the next tick after a rejected push', async () => { + const { root, ng } = fixture(); + g['ng'] = ng; + const session = fakeRpc(); + const scope = session.rpc.scope; + let rejected = false; + session.rpc.scope = () => { + const scoped = scope(); + const call = scoped.rpc.call; + scoped.rpc.call = vi.fn(async (name: string, arg: unknown) => { + const answer = await call(name, arg); + if (name === 'push-signal-graph' && !rejected) { + rejected = true; + throw new Error('socket down'); + } + return answer; + }); + return scoped; + }; + connectDevframe.mockResolvedValue(session.rpc); + const { initAngularNativeOverlay } = await import('../overlay-angular-native.ts'); + const dispose = initAngularNativeOverlay({ root: asNode(root), intervalMs: 1000 }); + await vi.waitFor(() => expect(rejected).toBe(true)); + await vi.advanceTimersByTimeAsync(2500); + const pushes = session.calls.filter(([name]) => name === 'push-signal-graph'); + expect(pushes.length).toBeGreaterThanOrEqual(2); + dispose(); + }); + it('outlines the node a highlight request names', async () => { const { root, second, ng } = fixture(); g['ng'] = ng; diff --git a/packages/devtools/src/__tests__/overlay-nativescript-report.test.ts b/packages/devtools/src/__tests__/overlay-nativescript-report.test.ts new file mode 100644 index 00000000..f6123ea7 --- /dev/null +++ b/packages/devtools/src/__tests__/overlay-nativescript-report.test.ts @@ -0,0 +1,111 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const calls: { name: string; at: number }[] = []; +let failTree = false; + +vi.mock('@nativescript/core', () => ({ + Application: { getRootView: () => null }, + isAndroid: false, +})); + +vi.mock('devframe/client', () => ({ + connectDevframe: vi.fn(async () => ({ + status: 'connected', + close: vi.fn(), + scope: () => ({ + rpc: { + call: vi.fn(async (name: string) => { + calls.push({ name, at: Date.now() }); + if (failTree && name === 'push-component-tree') throw new Error('rejected'); + return undefined; + }), + register: vi.fn(), + }, + }), + })), +})); + +vi.mock('../signal-history.ts', async (importOriginal) => ({ + ...(await importOriginal()), + installSignalWriteHook: vi.fn(async () => () => undefined), +})); + +vi.mock('../ngrx-overlay.ts', () => ({ + attachNgrx: () => ({ + push: async () => { + calls.push({ name: 'push-ngrx', at: Date.now() }); + }, + stop: () => undefined, + }), +})); + +vi.mock('../component-tree.ts', async (importOriginal) => ({ + ...(await importOriginal()), + collectComponentTree: () => ({ count: 0, roots: [] }), +})); + +vi.mock('../signal-graph.ts', async (importOriginal) => ({ + ...(await importOriginal()), + collectSignalGraph: () => ({ component: null, nodes: [] }), + graphKey: () => 'same', +})); + +vi.mock('../injector-tree.ts', async (importOriginal) => ({ + ...(await importOriginal()), + collectInjectorTree: () => ({ count: 0, roots: [] }), +})); + +async function flush() { + for (let i = 0; i < 30; i++) await Promise.resolve(); +} + +describe('NativeScript overlay reports', () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.stubGlobal('WebSocket', class {}); + vi.stubGlobal('ng', { getComponent: () => null }); + calls.length = 0; + failTree = false; + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it('keeps sending the other collectors when one push is rejected', async () => { + failTree = true; + const { initNativeScriptOverlay } = await import('../overlay-nativescript.ts'); + const dispose = initNativeScriptOverlay({ + baseURL: 'http://localhost:9999/', + intervalMs: 3000, + }); + await vi.advanceTimersByTimeAsync(0); + await flush(); + const names = calls.map((c) => c.name); + expect(names).toContain('push-component-tree'); + expect(names).toContain('push-signal-graph'); + expect(names).toContain('push-injector-tree'); + expect(names).toContain('push-ngrx'); + dispose(); + }); + + it('re-sends an unchanged tree within the page expiry at a slow interval', async () => { + const { initNativeScriptOverlay } = await import('../overlay-nativescript.ts'); + const dispose = initNativeScriptOverlay({ + baseURL: 'http://localhost:9999/', + intervalMs: 5000, + }); + await vi.advanceTimersByTimeAsync(0); + await flush(); + await vi.advanceTimersByTimeAsync(60_000); + const times = calls.filter((c) => c.name === 'push-component-tree').map((c) => c.at); + const gaps = times.slice(1).map((at, i) => at - times[i]); + expect(times.length).toBeGreaterThan(2); + expect(Math.max(...gaps)).toBeLessThanOrEqual(15_000); + dispose(); + }); +}); diff --git a/packages/devtools/src/__tests__/pipes-runtime.test.ts b/packages/devtools/src/__tests__/pipes-runtime.test.ts index b138ad57..1bfc80a9 100644 --- a/packages/devtools/src/__tests__/pipes-runtime.test.ts +++ b/packages/devtools/src/__tests__/pipes-runtime.test.ts @@ -110,6 +110,15 @@ Component({ template: `

{{ items().join(',') | uppercase }}

`, })(Cart); +class Labelled { + value = 'a'; +} +Component({ + selector: 'app-labelled', + imports: [UpperCasePipe], + template: `uppercase

{{ value | uppercase }}

`, +})(Labelled); + describe('stale pure-pipe check (experimental, real Angular render)', () => { it('recovers a real binding slot and reads the last argument Angular compared', async () => { const fixture = await mount(Cart); @@ -125,6 +134,16 @@ describe('stale pure-pipe check (experimental, real Angular render)', () => { expect(readBoundArg(upper, check!)).toBe('c,d'); }); + it('is not fooled by static text equal to the pipe name', async () => { + const fixture = await mount(Labelled); + const usages = findPipeUsages(ng(), document.querySelectorAll('*')); + const upper = usages.find((u) => u.name === 'uppercase')!; + const check = staleCheckFor(upper); + expect(check).not.toBeNull(); + expect(readBoundArg(upper, check!)).toBe('a'); + fixture.destroy(); + }); + it('returns null gracefully when the template source does not match the expected shape', () => { const fakeSlot = { name: 'uppercase', diff --git a/packages/devtools/src/__tests__/popup.test.ts b/packages/devtools/src/__tests__/popup.test.ts index a8ffd304..ea8e2eba 100644 --- a/packages/devtools/src/__tests__/popup.test.ts +++ b/packages/devtools/src/__tests__/popup.test.ts @@ -323,6 +323,18 @@ describe.sequential('devtools popup', () => { expect(stored().docked).toBe('float'); }); + it('draws no resize grip on a docked panel, whose size is fixed', async () => { + await loadPopup(true); + const sheet = document + .getElementById('pangular-popup-root')! + .shadowRoot!.querySelector('style')!.textContent!; + for (const mode of ['bottom', 'right']) { + const rule = new RegExp(`\\.panel\\.dock-${mode}\\s*\\{([^}]*)\\}`).exec(sheet)![1]; + expect(rule).toContain('!important'); + expect(rule).toMatch(/resize\s*:\s*none/); + } + }); + it('lets Escape clear a search box in the frame without closing the panel', async () => { await loadPopup(true); const doc = await frameDocument(); diff --git a/packages/devtools/src/__tests__/router-actions-probe-timeout.test.ts b/packages/devtools/src/__tests__/router-actions-probe-timeout.test.ts new file mode 100644 index 00000000..68c407c5 --- /dev/null +++ b/packages/devtools/src/__tests__/router-actions-probe-timeout.test.ts @@ -0,0 +1,70 @@ +// @vitest-environment jsdom +import '@angular/compiler'; +import { Component } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; +import { BrowserTestingModule, platformBrowserTesting } from '@angular/platform-browser/testing'; +import { Router, provideRouter, type Routes } from '@angular/router'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { watchRouter, type NavigationRecord } from '../router.ts'; +import { runAction } from '../router-actions.ts'; + +TestBed.initTestEnvironment(BrowserTestingModule, platformBrowserTesting()); + +class Page {} +Component({ selector: 'app-page', template: '' })(Page); + +let release: (value: boolean) => void = () => {}; +let guardedRuns = 0; + +const routes: Routes = [ + { path: '', component: Page }, + { + path: 'slow', + component: Page, + canMatch: [() => new Promise((resolve) => (release = resolve))], + canActivate: [ + () => { + guardedRuns++; + return true; + }, + ], + }, +]; + +describe('probe that times out', () => { + let router: Router; + let navigations: NavigationRecord[]; + let stop: (() => void) | null; + + beforeEach(async () => { + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ providers: [provideRouter(routes)] }); + router = TestBed.inject(Router); + navigations = []; + stop = watchRouter(router as never, navigations, () => {}); + await router.navigateByUrl('/'); + guardedRuns = 0; + }); + + afterEach(() => { + stop?.(); + vi.useRealTimers(); + }); + + it('aborts the probe navigation and marks it as a probe', async () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + const pending = runAction( + router as never, + navigations, + { action: 'probe', url: '/slow' }, + () => {}, + ); + await vi.advanceTimersByTimeAsync(10_000); + expect(await pending).toMatchObject({ error: expect.stringContaining('10s') }); + release(true); + await vi.advanceTimersByTimeAsync(50); + expect(router.url).toBe('/'); + expect(guardedRuns).toBe(0); + expect(navigations.some((n) => n.url === '/slow' && n.probe === true)).toBe(true); + }); +}); diff --git a/packages/devtools/src/__tests__/router-config-tools-gaps.test.ts b/packages/devtools/src/__tests__/router-config-tools-gaps.test.ts new file mode 100644 index 00000000..6c67c6da --- /dev/null +++ b/packages/devtools/src/__tests__/router-config-tools-gaps.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from 'vitest'; +import type { RouteNode } from '../router-config.ts'; +import { explainRenderModeText, lintRoutes } from '../rpc/router-config-tools.ts'; +import type { RouterPage } from '../rpc/router-tools.ts'; + +function node(fullPath: string, extra: Partial = {}): RouteNode { + const path = extra.path ?? fullPath.split('/').pop() ?? ''; + return { id: fullPath, path, fullPath, kind: 'component', component: 'C', ...extra }; +} + +function page(extra: Partial = {}): RouterPage { + return { pageId: 'p', snapshot: null, navigations: [], reportedAt: 1, changedAt: 1, ...extra }; +} + +describe('duplicate-path lint', () => { + it('does not flag same-path siblings that have different children', () => { + const config = [ + node('/', { path: '', children: [node('/a', { path: 'a' })] }), + node('/', { path: '', children: [node('/b', { path: 'b' })] }), + ]; + expect(lintRoutes(page({ config })).map((f) => f.rule)).not.toContain('duplicate-path'); + }); + + it('still flags identical leaf routes', () => { + const config = [node('/home'), node('/home')]; + expect(lintRoutes(page({ config })).filter((f) => f.rule === 'duplicate-path')).toHaveLength(1); + }); +}); + +describe('explain-render-mode', () => { + const entries = [{ path: 'admin/dashboard', renderMode: 'Server', file: 'x' }]; + + it('does not claim a build failure for entries under an unloaded lazy route', () => { + const config = [ + node('/admin', { kind: 'lazy', lazy: 'unloaded', component: undefined, path: 'admin' }), + ]; + const text = explainRenderModeText({ pages: [page({ config })] }, entries, {}); + expect(text).not.toContain('no matching client route'); + }); + + it('does not claim a build failure when the config was truncated', () => { + const text = explainRenderModeText( + { pages: [page({ config: [node('/other')], configTruncated: 3 })] }, + entries, + {}, + ); + expect(text).not.toContain('no matching client route'); + }); + + it('still reports an entry with no client route', () => { + const text = explainRenderModeText( + { pages: [page({ config: [node('/other')] })] }, + entries, + {}, + ); + expect(text).toContain('no matching client route'); + }); +}); diff --git a/packages/devtools/src/__tests__/router-gaps.test.ts b/packages/devtools/src/__tests__/router-gaps.test.ts new file mode 100644 index 00000000..af2a5647 --- /dev/null +++ b/packages/devtools/src/__tests__/router-gaps.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from 'vitest'; +import { walkConfig } from '../router-config.ts'; +import { detectSetup } from '../router-setup.ts'; +import { redirectCycles } from '../rpc/router-loops.ts'; +import type { RouteNode } from '../router-config.ts'; + +describe('redirectCycles', () => { + const node = (path: string, extra: Partial): RouteNode => ({ + id: path, + path, + fullPath: `/${path}`, + kind: 'redirect', + ...extra, + }); + + it('ignores redirects in a named outlet', () => { + const config = [ + node('a', { outlet: 'popup', redirectTo: 'b' }), + node('b', { redirectTo: 'a' }), + ]; + expect(redirectCycles(config)).toEqual([]); + }); + + it('still finds a cycle in the primary outlet', () => { + const config = [node('a', { redirectTo: 'b' }), node('b', { redirectTo: 'a' })]; + expect(redirectCycles(config)).toEqual([['/a', '/b', '/a']]); + }); +}); + +describe('walkConfig depth limit', () => { + it('counts the routes it leaves out below the depth limit', () => { + let route: Record = { path: 'leaf' }; + for (let i = 0; i < 14; i++) route = { path: `p${i}`, children: [route] }; + const cut = { routes: 0 }; + const nodes = walkConfig({ config: [route] }, cut); + let shown = 0; + const count = (list: RouteNode[]) => + list.forEach((n) => { + shown++; + count(n.children ?? []); + }); + count(nodes); + expect(cut.routes).toBeGreaterThan(0); + expect(shown + cut.routes).toBe(15); + }); +}); + +describe('router strategies', () => { + it('reports a missing strategy as unknown', () => { + const ng = { getInjector: () => undefined }; + const setup = detectSetup(ng as never, {} as never, 1, null); + expect(setup.strategies['locationStrategy']).toBe('unknown'); + expect(setup.strategies['titleStrategy']).toBe('unknown'); + }); +}); diff --git a/packages/devtools/src/cd-overlay.ts b/packages/devtools/src/cd-overlay.ts index c56c6598..8db0ab17 100644 --- a/packages/devtools/src/cd-overlay.ts +++ b/packages/devtools/src/cd-overlay.ts @@ -1,4 +1,4 @@ -import { addProfilerListener } from './change-detection.ts'; +import { addProfilerListener, keepaliveDue } from './change-detection.ts'; import { createCdRecorder, type CdRecording } from './cd-recorder.ts'; import { elementId } from './element-id.ts'; @@ -21,8 +21,6 @@ export interface CdReport extends CdRecording { supported: boolean; } -const KEEPALIVE_MS = 8000; - /** * Records change detection cycles while the panel or an agent turns recording * on, and reports them. It does nothing until then. @@ -32,6 +30,7 @@ export function attachChangeDetection( pageId: string, getNg: () => unknown, maxCycles: number, + tickMs: () => number = () => 0, ) { const recorder = createCdRecorder({ maxCycles }); const hostIds = new WeakMap(); @@ -71,7 +70,7 @@ export function attachChangeDetection( const report: CdReport = { pageId, supported, ...recorder.snapshot(hostId) }; const json = JSON.stringify(report); if (!force && json === lastJson) { - if (Date.now() - sentAt < KEEPALIVE_MS) return; + if (!keepaliveDue(sentAt, tickMs())) return; sentAt = Date.now(); const answer = (await my.rpc.call('ping-change-detection', pageId)) as { known?: boolean } | undefined; diff --git a/packages/devtools/src/forms-actions.ts b/packages/devtools/src/forms-actions.ts index adbefd83..d8d0159a 100644 --- a/packages/devtools/src/forms-actions.ts +++ b/packages/devtools/src/forms-actions.ts @@ -734,12 +734,16 @@ async function perform( else written++; } let message = `Wrote ${written} field(s)${skipped.length ? `, skipped ${skipped.length}` : ''}.`; - if (request.submit) { + let ok = written > 0 || !skipped.length; + if (request.submit && written === 0 && skipped.length) { + message += ' No field was written, so the form was not submitted.'; + } else if (request.submit) { await settle(ctx, found); const submitted = await perform(ctx, found, { ...request, action: 'submit' }, ''); message += ` ${submitted.message}`; + ok = ok && submitted.ok; } - return { ok: written > 0 || !skipped.length, message, skipped }; + return { ok, message, skipped }; } case 'mark-touched': case 'mark-untouched': diff --git a/packages/devtools/src/forms-dom.ts b/packages/devtools/src/forms-dom.ts index e85ad4f4..5f357c38 100644 --- a/packages/devtools/src/forms-dom.ts +++ b/packages/devtools/src/forms-dom.ts @@ -102,7 +102,10 @@ export function domFacts( facts.required = el.hasAttribute('required') || el.getAttribute('aria-required') === 'true' ? true : undefined; if (facts.required === undefined) delete facts.required; - facts.labelled = isLabelled(el); + const unlabelled = + (el instanceof HTMLInputElement && el.type === 'hidden') || + el.getAttribute('aria-hidden') === 'true'; + if (!unlabelled) facts.labelled = isLabelled(el); const describedBy = [ ...referenced(el, 'aria-describedby'), ...referenced(el, 'aria-errormessage'), diff --git a/packages/devtools/src/forms-instrument.ts b/packages/devtools/src/forms-instrument.ts index bb07c759..502a109c 100644 --- a/packages/devtools/src/forms-instrument.ts +++ b/packages/devtools/src/forms-instrument.ts @@ -49,7 +49,7 @@ export const VALIDATOR_METHODS = [ const METHODS = [...VALUE_METHODS, ...STRUCTURE_METHODS, ...STATE_METHODS, ...VALIDATOR_METHODS]; const FRAMEWORK = - /@angular|node_modules|\.vite\/deps|rxjs|zone\.js|zone-|forms-(instrument|collector|actions)\.[mc]?[jt]s|(packages|@pangular-inspector)\/devtools\/(dist|src\/(?!__tests__))||\(native\)/; + /@angular|node_modules|\.vite\/deps|(?|\(native\)/; const FRAMEWORK_CLASS = /^(Abstract|Form|Ng|Default|Operator|Checkbox|Select|Radio|Number|Range|Subscriber|Safe|Consumer|Observable|Subject|EventEmitter|Zone|Object|Array|Function|Proxy|Promise|Router|Renderer|Dom|EventManager|ApplicationRef|ChangeDetection|ViewRef|ComponentRef|Injector|R3Injector|NodeInjector|Listener|Task|Scheduler|Interop|Signal|Field|Deep)/; const FRAMEWORK_FUNCTION = diff --git a/packages/devtools/src/host-tree.ts b/packages/devtools/src/host-tree.ts index a139bf15..18eac0cd 100644 --- a/packages/devtools/src/host-tree.ts +++ b/packages/devtools/src/host-tree.ts @@ -60,6 +60,7 @@ export function angularRoots(doc: Document = document): Element[] { const roots = tagged.filter((root) => !tagged.some((o) => o !== root && o.contains(root))); if (!doc.body) return roots; if (!roots.length) return [doc.body]; + if (roots.includes(doc.body)) return roots; const outside: Element[] = []; const collect = (el: Element) => { for (const child of childElements(el)) { diff --git a/packages/devtools/src/ngrx-collector.ts b/packages/devtools/src/ngrx-collector.ts index f9b5694f..8a552089 100644 --- a/packages/devtools/src/ngrx-collector.ts +++ b/packages/devtools/src/ngrx-collector.ts @@ -994,6 +994,7 @@ export function createNgrxCollector( } } + for (const t of tracked.values()) t.references.clear(); for (const { injector, component } of perElement) { if (!component) continue; const owner = className(component.constructor); diff --git a/packages/devtools/src/overlay-angular-native.ts b/packages/devtools/src/overlay-angular-native.ts index 870bb8ba..b8303715 100644 --- a/packages/devtools/src/overlay-angular-native.ts +++ b/packages/devtools/src/overlay-angular-native.ts @@ -230,17 +230,36 @@ async function startSession( const owner = graph.component?.id ?? ''; const full = force || !historyDelta || owner !== historyFor; historyFor = owner; - const history = signalHistory.collectDelta(graph.nodes, full); - for (const item of graph.nodes) { + const statuses = (graph.resources ?? []) + .filter((r) => r.status) + .map((r) => ({ + id: r.id, + kind: 'resource' as const, + label: r.name, + epoch: r.epoch, + value: r.status, + })); + const { changes: history, rollback } = signalHistory.collectDeltaWithRollback( + [...graph.nodes, ...statuses], + full, + ); + for (const item of [...graph.nodes, ...(graph.resources ?? [])]) { const changes = signalHistory.changesOf(item.id); if (changes) item.changes = changes; } - const answer = (await my.rpc.call('push-signal-graph', { - ...graph, - ...(restoreSignalHook ? {} : { writeHook: false as const }), - pageId, - ...(full ? { history } : { historyDelta: history }), - })) as { delta?: boolean } | undefined; + let answer: { delta?: boolean } | undefined; + try { + answer = (await my.rpc.call('push-signal-graph', { + ...graph, + ...(restoreSignalHook ? {} : { writeHook: false as const }), + pageId, + ...(full ? { history } : { historyDelta: history }), + })) as { delta?: boolean } | undefined; + } catch (error) { + rollback(); + lastSignalKey = ''; + throw error; + } historyDelta = answer?.delta === true; } diff --git a/packages/devtools/src/overlay-nativescript.ts b/packages/devtools/src/overlay-nativescript.ts index d2a1c723..4f23dd41 100644 --- a/packages/devtools/src/overlay-nativescript.ts +++ b/packages/devtools/src/overlay-nativescript.ts @@ -8,6 +8,7 @@ import { Application, isAndroid } from '@nativescript/core'; import { connectDevframe } from 'devframe/client'; +import { keepaliveDue } from './change-detection.ts'; import { collectComponentTree, type ComponentDebugNg } from './component-tree.ts'; import { elementById } from './element-id.ts'; import { hostBySelector } from './host-tree.ts'; @@ -161,15 +162,15 @@ async function startSession( let componentTarget: string | null = null; let lastTreeJson = ''; - let treeSkips = 0; + let treeSentAt = 0; async function pushTree(force = false) { const ng = angularDebugApi(); if (!ng) return; const report = collectComponentTree(ng, { tree, selectedId: componentTarget }); const json = JSON.stringify(report); - if (!force && json === lastTreeJson && ++treeSkips < 4) return; + if (!force && json === lastTreeJson && !keepaliveDue(treeSentAt, intervalMs)) return; lastTreeJson = json; - treeSkips = 0; + treeSentAt = Date.now(); await my.rpc.call('push-component-tree', { ...report, pageId }); } @@ -180,16 +181,16 @@ async function startSession( let signalTarget: SignalTarget = null; let lastSignalKey = ''; - let signalSkips = 0; + let signalSentAt = 0; let historyDelta = false; let historyFor = ''; async function pushSignalGraph(force = false) { const graph = collectSignalGraph(angularDebugApi(), signalTarget, tree); if (!graph) return; const key = graphKey(graph); - if (!force && key === lastSignalKey && ++signalSkips < 4) return; + if (!force && key === lastSignalKey && !keepaliveDue(signalSentAt, intervalMs)) return; lastSignalKey = key; - signalSkips = 0; + signalSentAt = Date.now(); const owner = graph.component?.id ?? ''; const full = force || !historyDelta || owner !== historyFor; historyFor = owner; @@ -213,15 +214,15 @@ async function startSession( } let lastInjectorJson = ''; - let injectorSkips = 0; + let injectorSentAt = 0; async function pushInjectorTree() { const ng = angularDebugApi(); if (!ng) return; const report = collectInjectorTree(ng, tree); const json = JSON.stringify(report); - if (json === lastInjectorJson && ++injectorSkips < 4) return; + if (json === lastInjectorJson && !keepaliveDue(injectorSentAt, intervalMs)) return; lastInjectorJson = json; - injectorSkips = 0; + injectorSentAt = Date.now(); await my.rpc.call('push-injector-tree', { ...report, pageId }); } @@ -231,10 +232,9 @@ async function startSession( }); const report = async () => { - await pushTree(); - await pushSignalGraph(); - await pushInjectorTree(); - await ngrx.push(); + for (const collect of [pushTree, pushSignalGraph, pushInjectorTree, () => ngrx.push()]) { + await collect().catch((error: unknown) => console.warn('[pangular] report failed', error)); + } }; const tick = () => { if (rpc.status !== 'connected' && rpc.status !== 'connecting') return; diff --git a/packages/devtools/src/overlay.ts b/packages/devtools/src/overlay.ts index 4795ad22..8bb188cb 100644 --- a/packages/devtools/src/overlay.ts +++ b/packages/devtools/src/overlay.ts @@ -329,7 +329,9 @@ async function startOverlay(options: OverlayOptions, own: (cleanup: () => void) : null; const pipes = on.pipes ? attachPipes(my, pageId, getNg) : null; const http = on.http ? attachHttp(my, pageId, tickMs) : null; - const cd = on.components ? attachChangeDetection(my, pageId, getNg, limits.cdCycles) : null; + const cd = on.components + ? attachChangeDetection(my, pageId, getNg, limits.cdCycles, tickMs) + : null; const ngrx = on.ngrx ? attachNgrx(my, pageId, getNg, limits.changeLog) : null; const navigations: NavigationRecord[] = []; diff --git a/packages/devtools/src/pipes-runtime.ts b/packages/devtools/src/pipes-runtime.ts index 094f6bee..f6b124a9 100644 --- a/packages/devtools/src/pipes-runtime.ts +++ b/packages/devtools/src/pipes-runtime.ts @@ -325,10 +325,10 @@ export interface StaleCheck { * This recovers it by regex-scanning `tView.template.toString()` (available * in dev/unminified builds) for the pipe's own `ɵɵpipe(index, name)` call to * get its raw index, then for a call passing that same index as its first - * argument to get the binding's `slotOffset`. The pipe-creation call is - * matched by shape (`ident(N, 'name')`), but the binding call must be a - * `pipeBind` instruction so an unrelated call sharing the same first numeric - * argument can't supply the offset. It tolerates both AOT names + * argument to get the binding's `slotOffset`. The pipe-creation call must be a + * `pipe` instruction, so a static text node equal to the pipe name can't stand + * in for it, and the binding call must be a `pipeBind` instruction so an + * unrelated call sharing the same first numeric argument can't supply the offset. It tolerates both AOT names * (`ɵɵpipeBind1`) and JIT names (`jit___pipeBind1_8`). Returns `null` * whenever anything doesn't match; never throws. */ @@ -340,7 +340,10 @@ export function staleCheckFor(slot: PipeBinding): StaleCheck | null { if (typeof templateFn !== 'function' || typeof bindingRoot !== 'number') return null; const src = templateFn.toString(); const escapedName = slot.name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const creates = new RegExp(`\\w+\\(\\s*(\\d+)\\s*,\\s*['"\`]${escapedName}['"\`]\\s*\\)`, 'g'); + const creates = new RegExp( + `[\\w$\\u0275]*pipe(?:_\\d+)?\\(\\s*(\\d+)\\s*,\\s*['"\`]${escapedName}['"\`]\\s*[,)]`, + 'g', + ); const ordinal = ordinalOf(slot); let create: RegExpExecArray | null = null; for (let i = 0; i <= ordinal; i++) { diff --git a/packages/devtools/src/popup.ts b/packages/devtools/src/popup.ts index de9b0011..68e2794d 100644 --- a/packages/devtools/src/popup.ts +++ b/packages/devtools/src/popup.ts @@ -415,7 +415,7 @@ export function createDevtoolsPopup(options: { src?: string } = {}) { width: 100% !important; height: 40vh !important; border-radius: 10px 10px 0 0; - resize: vertical; + resize: none; } .panel.dock-right { top: 0 !important; @@ -425,7 +425,7 @@ export function createDevtoolsPopup(options: { src?: string } = {}) { width: 40vw !important; height: 100% !important; border-radius: 10px 0 0 10px; - resize: horizontal; + resize: none; } .toolbar { cursor: grab; diff --git a/packages/devtools/src/router-actions.ts b/packages/devtools/src/router-actions.ts index 487bdf97..3021247c 100644 --- a/packages/devtools/src/router-actions.ts +++ b/packages/devtools/src/router-actions.ts @@ -741,6 +741,8 @@ function probe(router: AnyRecord, navigations: NavigationRecord[], url: string): promise?.catch?.(() => {}); timer = setTimeout(() => { subscription.unsubscribe(); + abortCurrent(); + markProbe(targetId); resolve({ error: 'The probe did not finish within 10s.' }); }, WAIT_MS); }); diff --git a/packages/devtools/src/router-config.ts b/packages/devtools/src/router-config.ts index 32502501..da085233 100644 --- a/packages/devtools/src/router-config.ts +++ b/packages/devtools/src/router-config.ts @@ -98,7 +98,11 @@ export function walkConfig(router: AnyRecord, cut = { routes: 0 }): RouteNode[] prefix: string, depth: number, ): RouteNode[] => { - if (!Array.isArray(routes) || depth > MAX_DEPTH) return []; + if (!Array.isArray(routes)) return []; + if (depth > MAX_DEPTH) { + cut.routes += routes.reduce((total, route) => total + subtreeSize(route, 0), 0); + return []; + } const out: RouteNode[] = []; routes.forEach((route, index) => { if (index >= MAX_SIBLINGS || ++count > MAX_NODES) { diff --git a/packages/devtools/src/router-setup.ts b/packages/devtools/src/router-setup.ts index 34c0d9a7..c033b6e1 100644 --- a/packages/devtools/src/router-setup.ts +++ b/packages/devtools/src/router-setup.ts @@ -31,7 +31,9 @@ const OPTION_DEFAULTS: [string, string][] = [ ]; function strategyName(value: unknown): string { - return nameOf(value).replace(/^_+/, '') || 'unknown'; + if (value === undefined || value === null) return 'unknown'; + const name = nameOf(value); + return name.startsWith('[object') ? 'unknown' : name.replace(/^_+/, '') || 'unknown'; } export function preloaderOf(ng: RouterDebugApi, root: Element | null): AnyRecord | null { diff --git a/packages/devtools/src/rpc/__tests__/get-components-decorators.test.ts b/packages/devtools/src/rpc/__tests__/get-components-decorators.test.ts new file mode 100644 index 00000000..0b72bf5f --- /dev/null +++ b/packages/devtools/src/rpc/__tests__/get-components-decorators.test.ts @@ -0,0 +1,29 @@ +import { mkdirSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { fixtureDir } from './fixture-dir.ts'; +import { scan } from './scan.ts'; +import { getComponents } from '../get-components.ts'; + +async function componentsFor(source: string) { + const dir = fixtureDir('pangular-components-decorators-'); + mkdirSync(join(dir, 'src')); + writeFileSync(join(dir, 'src', 'widgets.ts'), source); + return scan(getComponents, dir); +} + +describe('get-components decorator members', () => { + it('keeps inputs and outputs whose options contain a closing parenthesis', async () => { + const [component] = await componentsFor(` + @Component({ selector: 'a', template: '' }) + export class A { + @Input({ transform: (v: string) => v.trim() }) name = ''; + @Input() other = ''; + @Output() changed = new EventEmitter(); + @Output(alias()) renamed = new EventEmitter(); + } + `); + expect(component.inputs).toEqual(['name', 'other']); + expect(component.outputs).toEqual(['changed', 'renamed']); + }); +}); diff --git a/packages/devtools/src/rpc/__tests__/get-ngrx-store-generics.test.ts b/packages/devtools/src/rpc/__tests__/get-ngrx-store-generics.test.ts new file mode 100644 index 00000000..2b0bbe4b --- /dev/null +++ b/packages/devtools/src/rpc/__tests__/get-ngrx-store-generics.test.ts @@ -0,0 +1,30 @@ +import { mkdirSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { fixtureDir } from './fixture-dir.ts'; +import { scan } from './scan.ts'; +import { getNgrxStore } from '../get-ngrx-store.ts'; + +async function storeFor(source: string) { + const dir = fixtureDir('pangular-ngrx-generics-'); + mkdirSync(join(dir, 'src')); + writeFileSync(join(dir, 'src', 'store.ts'), source); + return scan(getNgrxStore, dir); +} + +describe('get-ngrx-store generics', () => { + it('keeps a withState feature whose type argument has a comma', async () => { + const [store] = await storeFor( + `export const S = signalStore(withState>({ a: 1 }), withMethods(() => ({ go() {} })));`, + ); + expect(store.members?.state).toEqual(['a']); + expect(store.members?.methods).toEqual(['go']); + }); + + it('does not report generic arguments inside a value as members', async () => { + const [store] = await storeFor( + `export const S = signalStore(withState({ cache: new Map>(), other: 1 }));`, + ); + expect(store.members?.state).toEqual(['cache', 'other']); + }); +}); diff --git a/packages/devtools/src/rpc/__tests__/get-pipes-expressions.test.ts b/packages/devtools/src/rpc/__tests__/get-pipes-expressions.test.ts new file mode 100644 index 00000000..27acb5da --- /dev/null +++ b/packages/devtools/src/rpc/__tests__/get-pipes-expressions.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from 'vitest'; +import { pipeUsesIn } from '../get-pipes.ts'; + +describe('pipeUsesIn expression edge cases', () => { + it('ignores pipe-looking text inside a string literal', () => { + expect(pipeUsesIn(`

{{ 'x | json' }}

`)).toEqual([]); + expect(pipeUsesIn(`

`)).toEqual([]); + }); + + it('still reports a real pipe next to a string', () => { + expect(pipeUsesIn(`

{{ 'x' + v | json }}

`).map((u) => u.name)).toEqual(['json']); + }); + + it('reads pipes in animation and prefixed bindings', () => { + expect(pipeUsesIn(`
`).map((u) => u.name)).toEqual(['async']); + expect(pipeUsesIn(`
`).map((u) => u.name)).toEqual([ + 'json', + ]); + expect(pipeUsesIn(`
`).map((u) => u.name)).toEqual(['async']); + }); +}); diff --git a/packages/devtools/src/rpc/__tests__/get-routes-gaps.test.ts b/packages/devtools/src/rpc/__tests__/get-routes-gaps.test.ts new file mode 100644 index 00000000..e5d9f234 --- /dev/null +++ b/packages/devtools/src/rpc/__tests__/get-routes-gaps.test.ts @@ -0,0 +1,35 @@ +import { mkdirSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { fixtureDir } from './fixture-dir.ts'; +import { scan } from './scan.ts'; +import { getRoutes } from '../get-routes.ts'; + +async function routesFor(source: string) { + const dir = fixtureDir('pangular-routes-gaps-'); + mkdirSync(join(dir, 'src')); + writeFileSync(join(dir, 'src', 'app.routes.ts'), source); + return scan(getRoutes, dir); +} + +describe('get-routes extraction gaps', () => { + it('nests a child array referenced by name under the route that uses it', async () => { + const routes = await routesFor(` + const adminChildren: Routes = [{ path: 'users', component: Users }]; + export const routes: Routes = [{ path: 'admin', component: Admin, children: adminChildren }]; + `); + expect(routes.map((r) => r.fullPath).sort()).toEqual(['/admin', '/admin/users']); + }); + + it('keeps shorthand resolve entries', async () => { + const routes = await routesFor( + `[{ path: 'a', component: A, resolve: { user, org: orgResolver } }]`, + ); + expect(routes[0].resolvers).toEqual(['user: user', 'org: orgResolver']); + }); + + it('reads routes whose keys are quoted', async () => { + const routes = await routesFor(`[{ 'path': 'q', "component": Q }]`); + expect(routes.map((r) => [r.path, r.component])).toEqual([['q', 'Q']]); + }); +}); diff --git a/packages/devtools/src/rpc/__tests__/pipe-explain.test.ts b/packages/devtools/src/rpc/__tests__/pipe-explain.test.ts index 45ceb407..e0df37a6 100644 --- a/packages/devtools/src/rpc/__tests__/pipe-explain.test.ts +++ b/packages/devtools/src/rpc/__tests__/pipe-explain.test.ts @@ -35,6 +35,28 @@ describe('explainPipeText', () => { expect(live).not.toContain('—'); }); + it('shows already-described values as they are, without escaping them again', () => { + const live: PipesState = { + pipes: [ + { + name: 'appPrice', + className: 'PricePipe', + isPure: true, + instanceCount: 1, + components: [], + call: { callCount: 2, lastArgs: ['{"b":2}'], lastResult: '{"a":1}' }, + }, + ], + async: [], + reportedAt: 0, + instrumented: ['page-1'], + }; + const text = explainPipeText('appPrice', fixture(), live); + expect(text).toContain('Last input: `[{"b":2}]`'); + expect(text).toContain('Last output: `{"a":1}`'); + expect(text).not.toContain('\\"'); + }); + it('says the pipe was not seen when recording is on', () => { const text = explainPipeText('appPrice', fixture(), state(['page-1'])); expect(text).toContain('**Live:** not seen on the currently connected page.'); diff --git a/packages/devtools/src/rpc/__tests__/pipe-lint-for.test.ts b/packages/devtools/src/rpc/__tests__/pipe-lint-for.test.ts new file mode 100644 index 00000000..1a1cb341 --- /dev/null +++ b/packages/devtools/src/rpc/__tests__/pipe-lint-for.test.ts @@ -0,0 +1,32 @@ +import { mkdirSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { fixtureDir } from './fixture-dir.ts'; +import { lintPipes } from '../pipe-lint.ts'; + +function lintFor(template: string) { + const dir = fixtureDir('pangular-pipe-lint-for-'); + mkdirSync(join(dir, 'src')); + writeFileSync( + join(dir, 'src', 'app.ts'), + [ + `@Pipe({ name: 'slow', pure: false })`, + `export class SlowPipe {}`, + `@Component({ selector: 'a', template: \`${template}\` })`, + `export class A {}`, + ].join('\n'), + ); + return lintPipes(dir).filter((f) => f.rule === 'impure-pipe-in-for'); +} + +describe('impure-pipe-in-for block matching', () => { + it('does not flag a pipe after a compact loop', () => { + const template = `
    @for (i of items; track i) {
  • {{ i }}
    • }

{{ title | slow }}

`; + expect(lintFor(template)).toEqual([]); + }); + + it('reports a pipe in nested loops once', () => { + const template = `@for (r of rows; track r) {\n@for (c of r; track c) {\n{{ c | slow }}\n}\n}`; + expect(lintFor(template)).toHaveLength(1); + }); +}); diff --git a/packages/devtools/src/rpc/__tests__/source-scan-feature-folders.test.ts b/packages/devtools/src/rpc/__tests__/source-scan-feature-folders.test.ts new file mode 100644 index 00000000..47777221 --- /dev/null +++ b/packages/devtools/src/rpc/__tests__/source-scan-feature-folders.test.ts @@ -0,0 +1,27 @@ +import { mkdirSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { fixtureDir } from './fixture-dir.ts'; +import { scan } from './scan.ts'; +import { getComponents } from '../get-components.ts'; + +const component = (selector: string) => `@Component({ selector: '${selector}', template: '' }) +export class X {}`; + +describe('source scan feature folders', () => { + it('reads feature folders named like generated output, and still skips real output', async () => { + const dir = fixtureDir('pangular-feature-folders-'); + for (const folder of ['src/app/build', 'src/app/coverage', 'src/dist', 'src/node_modules/x']) { + mkdirSync(join(dir, folder), { recursive: true }); + writeFileSync( + join(dir, folder, 'x.component.ts'), + component(`app-${folder.replace(/\W/g, '-')}`), + ); + } + const components = await scan(getComponents, dir); + expect(components.map((c) => c.selector).sort()).toEqual([ + 'app-src-app-build', + 'app-src-app-coverage', + ]); + }); +}); diff --git a/packages/devtools/src/rpc/analog-tools.ts b/packages/devtools/src/rpc/analog-tools.ts index 0de46e48..ef4fdf05 100644 --- a/packages/devtools/src/rpc/analog-tools.ts +++ b/packages/devtools/src/rpc/analog-tools.ts @@ -232,20 +232,26 @@ function pickPage(state: AnalogState, page?: string): AnalogRuntimeReport | unde return page ? state.pages.find((p) => p.pageId === page) : state.pages[0]; } +const CONFIG_PATH_CAP = 500; + +function normalPath(path: string): string { + return `/${path.split('/').filter(Boolean).join('/')}`; +} + export function restartNeeded(project: AnalogProject, report: AnalogRuntimeReport): string[] { - const top = new Set( - report.configPaths.map( - (p) => `/${p.split('/').filter(Boolean)[0] ?? ''}`.replace(/\/$/, '') || '/', - ), - ); - if (!top.size) return []; - return project.routes - .filter((route) => route.file && route.kind !== 'group') - .map( - (route) => `/${route.fullPath.split('/').filter(Boolean)[0] ?? ''}`.replace(/\/$/, '') || '/', + if (!report.configPaths.length || report.configPaths.length >= CONFIG_PATH_CAP) return []; + const known = new Set(report.configPaths.map(normalPath)); + const missing = flattenRoutes(project.routes) + .filter( + (route) => + route.file && + (route.kind === 'page' || route.kind === 'markdown') && + !route.catchAll && + !route.fullPath.includes('**'), ) - .filter((path) => !top.has(path)) - .filter((path, i, all) => all.indexOf(path) === i); + .map((route) => normalPath(route.fullPath)) + .filter((path) => !known.has(path)); + return [...new Set(missing)]; } export function analogCurrentPageText( @@ -618,6 +624,20 @@ export function analogContentText(project: AnalogProject, filter?: string): stri return `${UNTRUSTED}\n\n${lines.join('\n')}`; } +function hasHandler(project: AnalogProject, call: AnalogCall): boolean { + const want = (call.route ?? call.url).split(/[?#]/)[0].split('/').filter(Boolean); + return project.api.some((api) => { + if (api.method !== 'ANY' && api.method !== call.method.toUpperCase()) return false; + const have = api.path.split('/').filter(Boolean); + for (let i = 0; i < have.length; i++) { + if (have[i] === '**') return i < want.length; + if (i >= want.length) return false; + if (!have[i].startsWith(':') && have[i] !== want[i]) return false; + } + return have.length === want.length; + }); +} + export function analogLint(project: AnalogProject, state: AnalogState): AnalogLintFinding[] { if (!project.analog) return []; const findings = lintAnalog(project); @@ -663,8 +683,12 @@ export function analogLint(project: AnalogProject, state: AnalogState): AnalogLi }); } } + const reported = new Set(); for (const call of state.calls) { if (call.kind === 'api' && (call.status === 404 || call.status === 405)) { + const key = `${call.method} ${call.route ?? call.url}`; + if (reported.has(key) || hasHandler(project, call)) continue; + reported.add(key); findings.push({ rule: 'api-not-found', severity: 'warning', diff --git a/packages/devtools/src/rpc/forms-explain.ts b/packages/devtools/src/rpc/forms-explain.ts index fa6062d6..cd0de6db 100644 --- a/packages/devtools/src/rpc/forms-explain.ts +++ b/packages/devtools/src/rpc/forms-explain.ts @@ -33,6 +33,17 @@ function walk(node: FormFieldNode, visit: (node: FormFieldNode) => void) { for (const child of node.children ?? []) walk(child, visit); } +function unmatchedForms( + state: FormsState, + args: FieldArgs, + matched: CollectedForm[], +): string | null { + if (matched.length || (!args.form && !args.page)) return null; + return state.forms.length + ? noMatch(onPage(state.forms, args.page), args.form ?? args.page ?? '') + : 'No forms have been reported.'; +} + function pickForm(state: FormsState, args: FieldArgs): CollectedForm | string { const forms = matchForms(state.forms, args.form, args.page); if (!forms.length) { @@ -378,6 +389,8 @@ export interface HistoryArgs extends FieldArgs { export function formHistoryText(state: FormsState, args: HistoryArgs): string { const forms = matchForms(state.forms, args.form, args.page); + const unmatched = unmatchedForms(state, args, forms); + if (unmatched) return unmatched; const ids = new Set(forms.map((f) => f.id)); const limit = Math.min(Math.max(args.limit ?? 50, 1), MAX_HISTORY); const events = state.events.filter( @@ -405,6 +418,8 @@ export function formHistoryText(state: FormsState, args: HistoryArgs): string { export function formDiffText(state: FormsState, args: FieldArgs & { since?: number }): string { const since = args.since ?? 0; const forms = matchForms(state.forms, args.form, args.page); + const unmatched = unmatchedForms(state, args, forms); + if (unmatched) return unmatched; const ids = new Set(forms.map((f) => f.id)); const net = new Map< string, diff --git a/packages/devtools/src/rpc/forms-lint.ts b/packages/devtools/src/rpc/forms-lint.ts index 7b4153f0..f0e67d04 100644 --- a/packages/devtools/src/rpc/forms-lint.ts +++ b/packages/devtools/src/rpc/forms-lint.ts @@ -200,12 +200,9 @@ export function lintForm( }); } - const submits = events.filter( - (event) => - event.formId === form.id && event.type === 'submit' && event.detail?.includes('INVALID'), - ); + const submits = events.filter((event) => event.formId === form.id && event.type === 'submit'); const last = submits[submits.length - 1]; - if (last && !/focus on (input|select|textarea)/.test(last.detail ?? '')) { + if (last?.detail?.includes('INVALID') && !/focus on (?!button\b|a\b)/.test(last.detail)) { add( 'no-focus-on-invalid-submit', 'info', diff --git a/packages/devtools/src/rpc/forms-source.ts b/packages/devtools/src/rpc/forms-source.ts index 9d4efc97..0010353a 100644 --- a/packages/devtools/src/rpc/forms-source.ts +++ b/packages/devtools/src/rpc/forms-source.ts @@ -1,6 +1,6 @@ import { readFileSync } from 'node:fs'; -import { relative } from 'node:path'; -import { lineCounter, sourceRoots, stripComments, walkFiles } from './source-scan.ts'; +import { dirname, join, relative } from 'node:path'; +import { lineCounter, skipString, sourceRoots, stripComments, walkFiles } from './source-scan.ts'; export interface SourceLine { file: string; @@ -77,7 +77,9 @@ function schemaRefs(text: string): string[] { function blockEnd(source: string, open: number): number { let depth = 0; for (let i = open; i < source.length; i++) { - if (source[i] === '(') depth++; + const ch = source[i]; + if (ch === '"' || ch === "'" || ch === '`') i = skipString(source, i); + else if (source[i] === '(') depth++; else if (source[i] === ')' && --depth === 0) return i; } return source.length; @@ -193,7 +195,7 @@ export function findFormSource( const content = readCached(full); if (content === null || !needle.test(content)) continue; result = formSourceIn(content, relative(cwd, full), owner, property, path, (name) => - schemaFile(cwd, files, name), + schemaFile(cwd, files, name, full, content), ); if (result) break; } @@ -201,13 +203,31 @@ export function findFormSource( return result; } -function schemaFile(cwd: string, files: string[], name: string): SchemaSource | null { +function schemaFile( + cwd: string, + files: string[], + name: string, + from: string, + fromContent: string, +): SchemaSource | null { const needle = new RegExp(`\\b(?:const|let|var)\\s+${escape(name)}\\b[^=;]*=\\s*schema\\b`); + const matches: { full: string; content: string }[] = []; for (const full of files) { const content = readCached(full); - if (content !== null && needle.test(content)) return { content, file: relative(cwd, full) }; + if (content !== null && needle.test(content)) matches.push({ full, content }); + } + const specifier = new RegExp( + `import\\s*\\{[^}]*\\b${escape(name)}\\b[^}]*\\}\\s*from\\s*['"](\\.[^'"]*)['"]`, + ).exec(stripComments(fromContent))?.[1]; + if (specifier) { + const base = join(dirname(from), specifier.replace(/\.[mc]?[jt]s$/, '')); + const imported = matches.find( + (match) => match.full === `${base}.ts` || match.full === join(base, 'index.ts'), + ); + if (imported) return { content: imported.content, file: relative(cwd, imported.full) }; } - return null; + if (matches.length !== 1) return null; + return { content: matches[0].content, file: relative(cwd, matches[0].full) }; } function readCached(full: string): string | null { diff --git a/packages/devtools/src/rpc/get-components.ts b/packages/devtools/src/rpc/get-components.ts index 69477263..633743ee 100644 --- a/packages/devtools/src/rpc/get-components.ts +++ b/packages/devtools/src/rpc/get-components.ts @@ -9,6 +9,7 @@ import { classScopes, lineCounter, maskStrings, + matchDelimiter, skipString, sourceRoots, stripComments, @@ -103,8 +104,8 @@ function componentsIn( kind, file: relPath, line: lineAt(scope.start), - inputs: [...names(body, INPUT), ...names(body, INPUT_DECORATOR)], - outputs: [...names(body, OUTPUT), ...names(body, OUTPUT_DECORATOR)], + inputs: [...names(body, INPUT), ...decoratedNames(body, 'Input')], + outputs: [...names(body, OUTPUT), ...decoratedNames(body, 'Output')], isStandalone: standaloneOf(scope.decoratorArgs, major), // A directive carries no change detection strategy of its own. ...(kind === 'component' ? changeDetectionFields(scope.decoratorArgs, major) : {}), @@ -240,11 +241,17 @@ const OUTPUT = new RegExp( // A member can carry modifiers and an accessor keyword before its name: // `@Input() set value(v)` declares `value`, not `set`. const MEMBER_PREFIX = String.raw`(?:(?:readonly|public|private|protected|override|declare|static|abstract|get|set|async)\s+)*`; -const INPUT_DECORATOR = new RegExp( - String.raw`@Input\([^)]*\)\s+` + MEMBER_PREFIX + String.raw`([$\w]+)`, - 'g', -); -const OUTPUT_DECORATOR = new RegExp( - String.raw`@Output\([^)]*\)\s+` + MEMBER_PREFIX + String.raw`([$\w]+)`, - 'g', -); +const DECORATED_MEMBER = new RegExp(String.raw`\s+` + MEMBER_PREFIX + String.raw`([$\w]+)`, 'y'); + +function decoratedNames(body: string, decorator: 'Input' | 'Output'): string[] { + const found: string[] = []; + const opening = new RegExp(String.raw`@${decorator}\(`, 'g'); + let match: RegExpExecArray | null; + while ((match = opening.exec(body)) !== null) { + const close = matchDelimiter(body, match.index + match[0].length - 1, '(', ')'); + DECORATED_MEMBER.lastIndex = close + 1; + const name = DECORATED_MEMBER.exec(body)?.[1]; + if (name) found.push(name); + } + return found; +} diff --git a/packages/devtools/src/rpc/get-ngrx-store.ts b/packages/devtools/src/rpc/get-ngrx-store.ts index bdcbcfd8..497fbe36 100644 --- a/packages/devtools/src/rpc/get-ngrx-store.ts +++ b/packages/devtools/src/rpc/get-ngrx-store.ts @@ -227,13 +227,27 @@ const FEATURES: Record = { withEntities: 'entities', }; +function genericEnd(text: string, open: number): number { + let angle = 0; + for (let i = open; i < text.length; i++) { + const ch = text[i]; + if (ch === ';') return -1; + if (ch === '<') angle++; + else if (ch === '>' && text[i - 1] !== '=' && --angle === 0) return i; + } + return -1; +} + function splitTop(text: string): { text: string; start: number }[] { const parts: { text: string; start: number }[] = []; let depth = 0; let start = 0; for (let i = 0; i < text.length; i++) { const ch = text[i]; - if (ch === '(' || ch === '[' || ch === '{') depth++; + if (ch === '<' && /[\w$>]/.test(text[i - 1] ?? '')) { + const close = genericEnd(text, i); + if (close > 0) i = close; + } else if (ch === '(' || ch === '[' || ch === '{') depth++; else if (ch === ')' || ch === ']' || ch === '}') depth--; else if (ch === ',' && depth === 0) { parts.push({ text: text.slice(start, i), start }); diff --git a/packages/devtools/src/rpc/get-pipes.ts b/packages/devtools/src/rpc/get-pipes.ts index 4fa3a121..8e48ea3d 100644 --- a/packages/devtools/src/rpc/get-pipes.ts +++ b/packages/devtools/src/rpc/get-pipes.ts @@ -311,7 +311,8 @@ function expressionRegionsIn(text: string): { start: number; end: number }[] { } // [prop]="…", [(prop)]="…", (event)="…", *directive="…" - const boundAttr = /(?:\[\(?[\w.$-]+\)?\]|\([\w.$-]+\)|\*[\w.$-]+)\s*=\s*(["'])/g; + const boundAttr = + /(?:\[\(?@?[\w.$-]+\)?\]|\(@?[\w.$-]+\)|\*[\w.$-]+|\b(?:bind|on|bindon)-[\w.$-]+)\s*=\s*(["'])/g; while ((m = boundAttr.exec(text)) !== null) { const quote = m[1]; const start = m.index + m[0].length; @@ -366,7 +367,7 @@ function letEnd(text: string, start: number): number { export function pipeUsesIn(text: string): { name: string; index: number }[] { const uses: { name: string; index: number }[] = []; for (const region of expressionRegionsIn(text)) { - const slice = text.slice(region.start, region.end); + const slice = maskStrings(text.slice(region.start, region.end)); PIPE_USE.lastIndex = 0; let match: RegExpExecArray | null; while ((match = PIPE_USE.exec(slice)) !== null) { diff --git a/packages/devtools/src/rpc/get-routes.ts b/packages/devtools/src/rpc/get-routes.ts index 829cb87a..f5979203 100644 --- a/packages/devtools/src/rpc/get-routes.ts +++ b/packages/devtools/src/rpc/get-routes.ts @@ -106,6 +106,7 @@ interface ParsedRoute { route: ExtractedRoute; parent: number; group?: string; + childrenRef?: string; lazy?: LazyTarget; } @@ -157,6 +158,8 @@ function parseFile(file: string, cwd: string): ParsedFile | null { parent: literal.parent >= 0 ? (kept.get(literal.parent) ?? -1) : -1, group: literal.group, }; + const childrenRef = props.get('children')?.match(/^[\w$]+$/)?.[0]; + if (childrenRef) parsed.childrenRef = childrenRef; const lazy = lazyTarget(props.get('loadChildren'), file, cwd); if (lazy) parsed.lazy = lazy; kept.set(index, routes.length); @@ -213,7 +216,16 @@ function resolveFiles(found: string[], cwd: string): ExtractedRoute[] { if (visiting.has(key)) return ''; visiting.add(key); const route = parsed.get(file)!.routes[index]; - const base = route.parent >= 0 ? fullPathOf(file, route.parent) : prefixOf(file, route.group); + const owner = + route.parent < 0 && route.group + ? parsed.get(file)!.routes.findIndex((r) => r.childrenRef === route.group) + : -1; + const base = + route.parent >= 0 + ? fullPathOf(file, route.parent) + : owner >= 0 + ? fullPathOf(file, owner) + : prefixOf(file, route.group); const full = joinPath(base, route.route.path); visiting.delete(key); memo.set(key, full); @@ -312,7 +324,7 @@ function routeGuards(props: Map): Record | und function routeResolvers(props: Map): string[] | undefined { const body = props.get('resolve')?.match(/^\{([\s\S]*)\}$/)?.[1]; if (body === undefined) return undefined; - const keys = [...topLevelProps(body)].map(([key, value]) => `${key}: ${guardName(value)}`); + const keys = [...topLevelProps(body, true)].map(([key, value]) => `${key}: ${guardName(value)}`); return keys.length ? keys : undefined; } @@ -447,11 +459,15 @@ function objectLiterals(source: string): RouteLiteral[] { return out; } -function topLevelProps(body: string): Map { +function topLevelProps(body: string, shorthand = false): Map { const props = new Map(); for (const part of splitTopLevel(body)) { - const prop = part.match(/^\s*(\w+)\s*:\s*([\s\S]*?)\s*$/); - if (prop) props.set(prop[1], prop[2]); + const prop = part.match(/^\s*(?:(['"])([\w$]+)\1|([\w$]+))\s*:\s*([\s\S]*?)\s*$/); + if (prop) props.set(prop[2] ?? prop[3], prop[4]); + else if (shorthand) { + const name = part.match(/^\s*([\w$]+)\s*$/)?.[1]; + if (name) props.set(name, name); + } } return props; } diff --git a/packages/devtools/src/rpc/pipe-explain.ts b/packages/devtools/src/rpc/pipe-explain.ts index 9bf300f8..627fdfee 100644 --- a/packages/devtools/src/rpc/pipe-explain.ts +++ b/packages/devtools/src/rpc/pipe-explain.ts @@ -11,7 +11,7 @@ function capJson(value: unknown): string { if (value === undefined) return 'undefined'; let json: string | undefined; try { - json = JSON.stringify(value); + json = typeof value === 'string' ? value : JSON.stringify(value); } catch { json = undefined; } @@ -19,6 +19,10 @@ function capJson(value: unknown): string { return text.length > MAX_VALUE_CHARS ? `${text.slice(0, MAX_VALUE_CHARS)}…` : text; } +function capArgs(args: unknown): string { + return Array.isArray(args) ? `[${args.map(capJson).join(', ')}]` : capJson(args); +} + /** Formats everything known about one pipe — static declaration/usage, live * call counts and last input/output (if instrumented), and any lint findings * — as markdown for an agent asking "why is this pipe slow or stale?" */ @@ -71,7 +75,7 @@ function runtimeText(runtime: PipeUsageInfo | undefined, instrumented: boolean): ]; if (runtime.call) { parts.push( - `Called ${runtime.call.callCount} time(s). Last input: ${code(capJson(runtime.call.lastArgs))}. Last output: ${code(capJson(runtime.call.lastResult))}.${ + `Called ${runtime.call.callCount} time(s). Last input: ${code(capArgs(runtime.call.lastArgs))}. Last output: ${code(capJson(runtime.call.lastResult))}.${ runtime.call.lastCaller ? ` Last caller: ${code(runtime.call.lastCaller)}.` : '' }`, ); @@ -81,7 +85,7 @@ function runtimeText(runtime: PipeUsageInfo | undefined, instrumented: boolean): `Per instance: ${instances .map( (i) => - `${i.component} ×${i.callCount} ${code(capJson(i.lastArgs))} → ${code(capJson(i.lastResult))}`, + `${i.component} ×${i.callCount} ${code(capArgs(i.lastArgs))} → ${code(capJson(i.lastResult))}`, ) .join('; ')}.`, ); diff --git a/packages/devtools/src/rpc/pipe-lint.ts b/packages/devtools/src/rpc/pipe-lint.ts index 72006b9e..c7946123 100644 --- a/packages/devtools/src/rpc/pipe-lint.ts +++ b/packages/devtools/src/rpc/pipe-lint.ts @@ -84,6 +84,15 @@ export function lintPipes(cwd: string): PipeLintFinding[] { return findings; } +function templateBraceEnd(text: string, open: number): number { + let depth = 0; + for (let i = open; i < text.length; i++) { + if (text[i] === '{') depth++; + else if (text[i] === '}' && --depth === 0) return i; + } + return text.length; +} + /** Byte spans of every `@for (...) { ... }` block's body in `text`. */ function forBlocks(text: string): { start: number; end: number }[] { const blocks: { start: number; end: number }[] = []; @@ -95,7 +104,7 @@ function forBlocks(text: string): { start: number; end: number }[] { if (closeParen >= text.length) break; const openBrace = text.indexOf('{', closeParen); if (openBrace === -1) break; - const closeBrace = matchDelimiter(text, openBrace, '{', '}'); + const closeBrace = templateBraceEnd(text, openBrace); blocks.push({ start: openBrace, end: closeBrace }); re.lastIndex = openBrace + 1; } @@ -104,9 +113,12 @@ function forBlocks(text: string): { start: number; end: number }[] { function impurePipeInForFindings(template: TemplateSource, purity: PurityMap): PipeLintFinding[] { const findings: PipeLintFinding[] = []; + const seen = new Set(); for (const block of forBlocks(template.text)) { const body = template.text.slice(block.start, block.end); for (const { name, index } of pipeUsesIn(body)) { + if (seen.has(block.start + index)) continue; + seen.add(block.start + index); const isPure = purity.get(name); if (isPure === undefined || isPure) continue; findings.push({ diff --git a/packages/devtools/src/rpc/router-config-tools.ts b/packages/devtools/src/rpc/router-config-tools.ts index c54c7d35..2eebae2a 100644 --- a/packages/devtools/src/rpc/router-config-tools.ts +++ b/packages/devtools/src/rpc/router-config-tools.ts @@ -343,7 +343,16 @@ export function lintRoutes(page: RouterPage): LintFinding[] { for (const node of primary) { const key = `${node.path}|${node.pathMatch ?? 'prefix'}`; const earlier = seen.get(key); - if (earlier && !node.guards?.canMatch && !earlier.guards?.canMatch && !node.matcher) { + if ( + earlier && + !node.guards?.canMatch && + !earlier.guards?.canMatch && + !node.matcher && + !node.children?.length && + !earlier.children?.length && + node.lazy !== 'unloaded' && + earlier.lazy !== 'unloaded' + ) { findings.push({ rule: 'duplicate-path', severity: 'warning', @@ -753,12 +762,18 @@ export function explainRenderModeText( } if (page?.config) { const clientPaths: string[] = []; + const unloaded: string[] = []; walk(page.config, (node) => { if (!node.children?.length && node.redirectTo === undefined) clientPaths.push(node.fullPath); + if (node.lazy === 'unloaded') unloaded.push(node.fullPath); }); const unknown = entries.filter( (entry) => entry.path !== '**' && + !page.configTruncated && + !unloaded.some((prefix) => + serverPathMatches(`${prefix.replace(/^\//, '')}/**`, `/${entry.path}`), + ) && !clientPaths.some( (path) => serverPathMatches(entry.path, path) || diff --git a/packages/devtools/src/rpc/router-loops.ts b/packages/devtools/src/rpc/router-loops.ts index ac75ea81..e6484900 100644 --- a/packages/devtools/src/rpc/router-loops.ts +++ b/packages/devtools/src/rpc/router-loops.ts @@ -57,7 +57,12 @@ function keyOf(url: string): string { export function redirectCycles(config: RouteNode[]): string[][] { const edges = new Map(); walk(config, (node, parents) => { - if (typeof node.redirectTo !== 'string' || node.redirectTo.startsWith('function ')) return; + if ( + node.outlet || + typeof node.redirectTo !== 'string' || + node.redirectTo.startsWith('function ') + ) + return; if (/:/.test(node.path) || node.path === '**') return; const base = parents.length ? parents[parents.length - 1].fullPath : ''; const target = node.redirectTo.startsWith('/') diff --git a/packages/devtools/src/rpc/source-scan.ts b/packages/devtools/src/rpc/source-scan.ts index 338b642d..72a96b40 100644 --- a/packages/devtools/src/rpc/source-scan.ts +++ b/packages/devtools/src/rpc/source-scan.ts @@ -406,6 +406,14 @@ export const IGNORED_DIRS = new Set([ '.yarn', ]); +const GENERATED_DIRS = new Set(['dist', 'build', 'out-tsc', 'coverage', 'tmp']); + +function skipsDirectory(name: string, top: boolean): boolean { + const lower = name.toLowerCase(); + if (!IGNORED_DIRS.has(lower)) return false; + return top || !GENERATED_DIRS.has(lower); +} + /** * Calls `visit` with the path and name of every file under `dir`, skipping * `IGNORED_DIRS` and unreadable entries. Symbolic links are not followed: a @@ -417,6 +425,7 @@ export function walkFiles( dir: string, visit: (full: string, name: string) => boolean | void, maxDepth = Infinity, + top = true, ): boolean { let entries: string[]; try { @@ -430,8 +439,8 @@ export function walkFiles( const stats = lstatSync(full); if (stats.isSymbolicLink()) continue; if (stats.isDirectory()) { - if (maxDepth > 0 && !IGNORED_DIRS.has(entry.toLowerCase())) { - if (!walkFiles(full, visit, maxDepth - 1)) return false; + if (maxDepth > 0 && !skipsDirectory(entry, top)) { + if (!walkFiles(full, visit, maxDepth - 1, false)) return false; } continue; } From 92652f117c8a5ffb7947891bb23d8172aca4ddb1 Mon Sep 17 00:00:00 2001 From: Kam Date: Fri, 9 Oct 2026 22:48:05 +0300 Subject: [PATCH 2/2] fix: address review findings on the P3 collector and scanner fixes A rejected Angular Native history push now resets the delta flag so the retry is a full push. A rejected NativeScript tree or injector push restores the sent state so the next tick retries. Schema lookup matches the local binding of an import and returns nothing for an unresolved import. The @for body ignores braces inside interpolation strings. Duplicate-path keeps comparing against earlier leaf routes, and only unloaded loadChildren routes suppress the render mode warning. Nested roots inside generated folders are no longer scanned twice, and the docs count test rejects an unknown number word. --- .../src/__tests__/docs-tool-count.test.ts | 4 +- .../src/__tests__/forms-source-gaps.test.ts | 31 ++++++++++++++++ .../__tests__/overlay-angular-native.test.ts | 37 +++++++++++++++++++ .../overlay-nativescript-report.test.ts | 23 ++++++++++++ .../router-config-tools-gaps.test.ts | 17 +++++++++ .../devtools/src/overlay-angular-native.ts | 1 + packages/devtools/src/overlay-nativescript.ts | 22 ++++++++++- .../src/rpc/__tests__/pipe-lint-for.test.ts | 5 +++ .../source-scan-feature-folders.test.ts | 14 +++++++ packages/devtools/src/rpc/forms-source.ts | 24 +++++++++--- packages/devtools/src/rpc/pipe-lint.ts | 13 ++++++- .../devtools/src/rpc/router-config-tools.ts | 8 ++-- packages/devtools/src/rpc/source-scan.ts | 2 +- 13 files changed, 187 insertions(+), 14 deletions(-) diff --git a/packages/devtools/src/__tests__/docs-tool-count.test.ts b/packages/devtools/src/__tests__/docs-tool-count.test.ts index ce064932..bb59f23e 100644 --- a/packages/devtools/src/__tests__/docs-tool-count.test.ts +++ b/packages/devtools/src/__tests__/docs-tool-count.test.ts @@ -30,5 +30,7 @@ it('states on the tools page how many tools the server registers', async () => { ); const stated = /^\s*([A-Z][a-z]+(?:-[a-z]+)?) tools, grouped by inspector/m.exec(page)?.[1]; expect(stated).toBeDefined(); - expect(WORDS.indexOf(stated!) + 48).toBe(registered); + const index = WORDS.indexOf(stated!); + expect(index).toBeGreaterThanOrEqual(0); + expect(index + 48).toBe(registered); }); diff --git a/packages/devtools/src/__tests__/forms-source-gaps.test.ts b/packages/devtools/src/__tests__/forms-source-gaps.test.ts index f4ad1890..06ccfa86 100644 --- a/packages/devtools/src/__tests__/forms-source-gaps.test.ts +++ b/packages/devtools/src/__tests__/forms-source-gaps.test.ts @@ -44,6 +44,37 @@ function other() { expect(found?.schemas?.map((s) => s.file.replaceAll('\\', '/'))).toEqual(['src/b/schema.ts']); }); + it('matches the local binding of an aliased import', () => { + const cwd = mkdtempSync(join(tmpdir(), 'pangular-schema-')); + for (const dir of ['a', 'b']) { + mkdirSync(join(cwd, 'src', dir), { recursive: true }); + writeFileSync( + join(cwd, 'src', dir, 'schema.ts'), + `export const addressSchema = schema((p) => {\n required(p.city);\n});\n`, + ); + } + writeFileSync( + join(cwd, 'src', 'page.ts'), + `import { addressSchema as shippingSchema } from './a/schema';\nimport { addressSchema } from './b/schema';\nexport class Page {\n f = form(this.model, addressSchema);\n}\n`, + ); + const found = findFormSource(cwd, 'Page', 'f', 'city'); + expect(found?.schemas?.map((s) => s.file.replaceAll('\\', '/'))).toEqual(['src/b/schema.ts']); + }); + + it('does not use an unrelated declaration when the import cannot be resolved', () => { + const cwd = mkdtempSync(join(tmpdir(), 'pangular-schema-')); + mkdirSync(join(cwd, 'src', 'a'), { recursive: true }); + writeFileSync( + join(cwd, 'src', 'a', 'schema.ts'), + `export const addressSchema = schema((p) => {\n required(p.city);\n});\n`, + ); + writeFileSync( + join(cwd, 'src', 'page.ts'), + `import { addressSchema } from '@app/schema';\nexport class Page {\n f = form(this.model, addressSchema);\n}\n`, + ); + expect(findFormSource(cwd, 'Page', 'f', 'city')?.schemas).toBeUndefined(); + }); + it('does not guess between same-named schemas it cannot tell apart', () => { const cwd = mkdtempSync(join(tmpdir(), 'pangular-schema-')); for (const dir of ['a', 'b']) { diff --git a/packages/devtools/src/__tests__/overlay-angular-native.test.ts b/packages/devtools/src/__tests__/overlay-angular-native.test.ts index 76ce1682..d734beb6 100644 --- a/packages/devtools/src/__tests__/overlay-angular-native.test.ts +++ b/packages/devtools/src/__tests__/overlay-angular-native.test.ts @@ -470,6 +470,43 @@ describe('initAngularNativeOverlay', () => { dispose(); }); + it('retries a rejected full history push as a full push, not a delta', async () => { + const { root, ng } = fixture(); + g['ng'] = ng; + const session = fakeRpc(); + const scope = session.rpc.scope; + let failNext = false; + session.rpc.scope = () => { + const scoped = scope(); + const call = scoped.rpc.call; + scoped.rpc.call = vi.fn(async (name: string, arg: unknown) => { + const answer = await call(name, arg); + if (name === 'push-signal-graph' && failNext) { + failNext = false; + throw new Error('socket down'); + } + return (name === 'push-signal-graph' ? { delta: true } : answer) as typeof answer; + }); + return scoped; + }; + connectDevframe.mockResolvedValue(session.rpc); + const { initAngularNativeOverlay } = await import('../overlay-angular-native.ts'); + const dispose = initAngularNativeOverlay({ root: asNode(root), intervalMs: 1000 }); + await vi.waitFor(() => expect(session.handlers.has('select-signal-component')).toBe(true)); + await vi.waitFor(() => + expect(session.calls.some(([name]) => name === 'push-signal-graph')).toBe(true), + ); + failNext = true; + session.handlers.get('select-signal-component')?.({ selector: 'app-row' }); + await vi.advanceTimersByTimeAsync(1500); + const pushes = session.calls.filter(([name]) => name === 'push-signal-graph'); + const retry = pushes.at(-1)?.[1] as Record; + expect(pushes.length).toBeGreaterThanOrEqual(3); + expect(retry).toHaveProperty('history'); + expect(retry).not.toHaveProperty('historyDelta'); + dispose(); + }); + it('outlines the node a highlight request names', async () => { const { root, second, ng } = fixture(); g['ng'] = ng; diff --git a/packages/devtools/src/__tests__/overlay-nativescript-report.test.ts b/packages/devtools/src/__tests__/overlay-nativescript-report.test.ts index f6123ea7..be22165e 100644 --- a/packages/devtools/src/__tests__/overlay-nativescript-report.test.ts +++ b/packages/devtools/src/__tests__/overlay-nativescript-report.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const calls: { name: string; at: number }[] = []; let failTree = false; +let failOnce: string | null = null; vi.mock('@nativescript/core', () => ({ Application: { getRootView: () => null }, @@ -17,6 +18,10 @@ vi.mock('devframe/client', () => ({ call: vi.fn(async (name: string) => { calls.push({ name, at: Date.now() }); if (failTree && name === 'push-component-tree') throw new Error('rejected'); + if (failOnce === name) { + failOnce = null; + throw new Error('rejected'); + } return undefined; }), register: vi.fn(), @@ -66,6 +71,7 @@ describe('NativeScript overlay reports', () => { vi.stubGlobal('ng', { getComponent: () => null }); calls.length = 0; failTree = false; + failOnce = null; vi.spyOn(console, 'log').mockImplementation(() => undefined); vi.spyOn(console, 'warn').mockImplementation(() => undefined); }); @@ -93,6 +99,23 @@ describe('NativeScript overlay reports', () => { dispose(); }); + it.each(['push-component-tree', 'push-injector-tree'])( + 'retries an unchanged %s on the next tick after it was rejected', + async (name) => { + failOnce = name; + const { initNativeScriptOverlay } = await import('../overlay-nativescript.ts'); + const dispose = initNativeScriptOverlay({ + baseURL: 'http://localhost:9999/', + intervalMs: 3000, + }); + await vi.advanceTimersByTimeAsync(0); + await flush(); + await vi.advanceTimersByTimeAsync(3000); + expect(calls.filter((c) => c.name === name)).toHaveLength(2); + dispose(); + }, + ); + it('re-sends an unchanged tree within the page expiry at a slow interval', async () => { const { initNativeScriptOverlay } = await import('../overlay-nativescript.ts'); const dispose = initNativeScriptOverlay({ diff --git a/packages/devtools/src/__tests__/router-config-tools-gaps.test.ts b/packages/devtools/src/__tests__/router-config-tools-gaps.test.ts index 6c67c6da..280ea09a 100644 --- a/packages/devtools/src/__tests__/router-config-tools-gaps.test.ts +++ b/packages/devtools/src/__tests__/router-config-tools-gaps.test.ts @@ -21,6 +21,15 @@ describe('duplicate-path lint', () => { expect(lintRoutes(page({ config })).map((f) => f.rule)).not.toContain('duplicate-path'); }); + it('keeps comparing against an earlier leaf when a middle sibling has children', () => { + const config = [ + node('/home'), + node('/home', { children: [node('/home/x', { path: 'x' })] }), + node('/home'), + ]; + expect(lintRoutes(page({ config })).filter((f) => f.rule === 'duplicate-path')).toHaveLength(1); + }); + it('still flags identical leaf routes', () => { const config = [node('/home'), node('/home')]; expect(lintRoutes(page({ config })).filter((f) => f.rule === 'duplicate-path')).toHaveLength(1); @@ -38,6 +47,14 @@ describe('explain-render-mode', () => { expect(text).not.toContain('no matching client route'); }); + it('still reports an entry under a route that only loads a component lazily', () => { + const config = [ + node('/admin', { kind: 'component', lazy: 'unloaded', component: undefined, path: 'admin' }), + ]; + const text = explainRenderModeText({ pages: [page({ config })] }, entries, {}); + expect(text).toContain('no matching client route'); + }); + it('does not claim a build failure when the config was truncated', () => { const text = explainRenderModeText( { pages: [page({ config: [node('/other')], configTruncated: 3 })] }, diff --git a/packages/devtools/src/overlay-angular-native.ts b/packages/devtools/src/overlay-angular-native.ts index b8303715..494ebef6 100644 --- a/packages/devtools/src/overlay-angular-native.ts +++ b/packages/devtools/src/overlay-angular-native.ts @@ -258,6 +258,7 @@ async function startSession( } catch (error) { rollback(); lastSignalKey = ''; + historyDelta = false; throw error; } historyDelta = answer?.delta === true; diff --git a/packages/devtools/src/overlay-nativescript.ts b/packages/devtools/src/overlay-nativescript.ts index 4f23dd41..0a85347f 100644 --- a/packages/devtools/src/overlay-nativescript.ts +++ b/packages/devtools/src/overlay-nativescript.ts @@ -169,9 +169,18 @@ async function startSession( const report = collectComponentTree(ng, { tree, selectedId: componentTarget }); const json = JSON.stringify(report); if (!force && json === lastTreeJson && !keepaliveDue(treeSentAt, intervalMs)) return; + const previous = { json: lastTreeJson, sentAt: treeSentAt }; lastTreeJson = json; treeSentAt = Date.now(); - await my.rpc.call('push-component-tree', { ...report, pageId }); + try { + await my.rpc.call('push-component-tree', { ...report, pageId }); + } catch (error) { + if (lastTreeJson === json) { + lastTreeJson = previous.json; + treeSentAt = previous.sentAt; + } + throw error; + } } const signalHistory = createSignalHistory((value, name) => @@ -221,9 +230,18 @@ async function startSession( const report = collectInjectorTree(ng, tree); const json = JSON.stringify(report); if (json === lastInjectorJson && !keepaliveDue(injectorSentAt, intervalMs)) return; + const previous = { json: lastInjectorJson, sentAt: injectorSentAt }; lastInjectorJson = json; injectorSentAt = Date.now(); - await my.rpc.call('push-injector-tree', { ...report, pageId }); + try { + await my.rpc.call('push-injector-tree', { ...report, pageId }); + } catch (error) { + if (lastInjectorJson === json) { + lastInjectorJson = previous.json; + injectorSentAt = previous.sentAt; + } + throw error; + } } const ngrx = attachNgrx(my, pageId, angularDebugApi, undefined, { diff --git a/packages/devtools/src/rpc/__tests__/pipe-lint-for.test.ts b/packages/devtools/src/rpc/__tests__/pipe-lint-for.test.ts index 1a1cb341..b9886b12 100644 --- a/packages/devtools/src/rpc/__tests__/pipe-lint-for.test.ts +++ b/packages/devtools/src/rpc/__tests__/pipe-lint-for.test.ts @@ -25,6 +25,11 @@ describe('impure-pipe-in-for block matching', () => { expect(lintFor(template)).toEqual([]); }); + it('ignores a brace inside a string when finding the end of the loop body', () => { + const template = `@for (x of xs; track x) { {{ '}' }} {{ x | slow }} }`; + expect(lintFor(template)).toHaveLength(1); + }); + it('reports a pipe in nested loops once', () => { const template = `@for (r of rows; track r) {\n@for (c of r; track c) {\n{{ c | slow }}\n}\n}`; expect(lintFor(template)).toHaveLength(1); diff --git a/packages/devtools/src/rpc/__tests__/source-scan-feature-folders.test.ts b/packages/devtools/src/rpc/__tests__/source-scan-feature-folders.test.ts index 47777221..a06a23d1 100644 --- a/packages/devtools/src/rpc/__tests__/source-scan-feature-folders.test.ts +++ b/packages/devtools/src/rpc/__tests__/source-scan-feature-folders.test.ts @@ -24,4 +24,18 @@ describe('source scan feature folders', () => { 'app-src-app-coverage', ]); }); + + it('scans a project rooted in a feature folder only once', async () => { + const dir = fixtureDir('pangular-nested-roots-'); + mkdirSync(join(dir, 'src/app/build'), { recursive: true }); + writeFileSync(join(dir, 'src/app/build/x.component.ts'), component('app-build')); + writeFileSync( + join(dir, 'angular.json'), + JSON.stringify({ + projects: { web: { sourceRoot: 'src' }, tools: { sourceRoot: 'src/app/build' } }, + }), + ); + const components = await scan(getComponents, dir); + expect(components.map((c) => c.selector)).toEqual(['app-build']); + }); }); diff --git a/packages/devtools/src/rpc/forms-source.ts b/packages/devtools/src/rpc/forms-source.ts index 0010353a..c485a5ae 100644 --- a/packages/devtools/src/rpc/forms-source.ts +++ b/packages/devtools/src/rpc/forms-source.ts @@ -203,6 +203,21 @@ export function findFormSource( return result; } +function importSpecifier(source: string, local: string): string | undefined { + for (const clause of source.matchAll(/import\s*\{([^}]*)\}\s*from\s*['"]([^'"]+)['"]/g)) { + for (const part of clause[1].split(',')) { + const bound = part + .trim() + .replace(/^type\s+/, '') + .split(/\s+as\s+/) + .pop() + ?.trim(); + if (bound === local) return clause[2]; + } + } + return undefined; +} + function schemaFile( cwd: string, files: string[], @@ -216,15 +231,14 @@ function schemaFile( const content = readCached(full); if (content !== null && needle.test(content)) matches.push({ full, content }); } - const specifier = new RegExp( - `import\\s*\\{[^}]*\\b${escape(name)}\\b[^}]*\\}\\s*from\\s*['"](\\.[^'"]*)['"]`, - ).exec(stripComments(fromContent))?.[1]; - if (specifier) { + const specifier = importSpecifier(stripComments(fromContent), name); + if (specifier !== undefined) { + if (!specifier.startsWith('.')) return null; const base = join(dirname(from), specifier.replace(/\.[mc]?[jt]s$/, '')); const imported = matches.find( (match) => match.full === `${base}.ts` || match.full === join(base, 'index.ts'), ); - if (imported) return { content: imported.content, file: relative(cwd, imported.full) }; + return imported ? { content: imported.content, file: relative(cwd, imported.full) } : null; } if (matches.length !== 1) return null; return { content: matches[0].content, file: relative(cwd, matches[0].full) }; diff --git a/packages/devtools/src/rpc/pipe-lint.ts b/packages/devtools/src/rpc/pipe-lint.ts index c7946123..8ab34995 100644 --- a/packages/devtools/src/rpc/pipe-lint.ts +++ b/packages/devtools/src/rpc/pipe-lint.ts @@ -6,6 +6,7 @@ import { lineCounter, maskStrings, matchDelimiter, + skipString, sourceRoots, stripComments, walkFiles, @@ -84,10 +85,20 @@ export function lintPipes(cwd: string): PipeLintFinding[] { return findings; } +function interpolationEnd(text: string, open: number): number { + for (let i = open + 2; i < text.length; i++) { + const ch = text[i]; + if (ch === '"' || ch === "'" || ch === '`') i = skipString(text, i); + else if (text.startsWith('}}', i)) return i + 1; + } + return text.length; +} + function templateBraceEnd(text: string, open: number): number { let depth = 0; for (let i = open; i < text.length; i++) { - if (text[i] === '{') depth++; + if (text.startsWith('{{', i)) i = interpolationEnd(text, i); + else if (text[i] === '{') depth++; else if (text[i] === '}' && --depth === 0) return i; } return text.length; diff --git a/packages/devtools/src/rpc/router-config-tools.ts b/packages/devtools/src/rpc/router-config-tools.ts index 2eebae2a..352c410d 100644 --- a/packages/devtools/src/rpc/router-config-tools.ts +++ b/packages/devtools/src/rpc/router-config-tools.ts @@ -350,8 +350,8 @@ export function lintRoutes(page: RouterPage): LintFinding[] { !node.matcher && !node.children?.length && !earlier.children?.length && - node.lazy !== 'unloaded' && - earlier.lazy !== 'unloaded' + !(node.kind === 'lazy' && node.lazy === 'unloaded') && + !(earlier.kind === 'lazy' && earlier.lazy === 'unloaded') ) { findings.push({ rule: 'duplicate-path', @@ -362,7 +362,7 @@ export function lintRoutes(page: RouterPage): LintFinding[] { angular: 'silent', }); } - seen.set(key, node); + if (!node.children?.length) seen.set(key, node); } primary.forEach((node, index) => { const own = parts(node.path); @@ -765,7 +765,7 @@ export function explainRenderModeText( const unloaded: string[] = []; walk(page.config, (node) => { if (!node.children?.length && node.redirectTo === undefined) clientPaths.push(node.fullPath); - if (node.lazy === 'unloaded') unloaded.push(node.fullPath); + if (node.kind === 'lazy' && node.lazy === 'unloaded') unloaded.push(node.fullPath); }); const unknown = entries.filter( (entry) => diff --git a/packages/devtools/src/rpc/source-scan.ts b/packages/devtools/src/rpc/source-scan.ts index 72a96b40..9473e8af 100644 --- a/packages/devtools/src/rpc/source-scan.ts +++ b/packages/devtools/src/rpc/source-scan.ts @@ -376,7 +376,7 @@ export function sourceRoots(cwd: string): string[] { // declared below one is only reachable by starting there. const crosses = relative(cover, real) .split(/[\\/]/) - .some((part) => IGNORED_DIRS.has(part.toLowerCase())); + .some((part, depth) => skipsDirectory(part, depth === 0)); if (!crosses) continue; kept.push(dir); continue;