From 82b695918780b4f88483487af53485032d1a1094 Mon Sep 17 00:00:00 2001 From: Artem Belik Date: Wed, 2 Sep 2026 18:13:42 +0300 Subject: [PATCH 1/2] fix(link)!: errors following a full review of the component MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three inputs the automated signal migration skipped were all accessors, and each did something beyond storing a value: `disabled` wrote a separate signal, `tabIndex` folded in the disabled state, and `print` was a setter with no getter that also computed the printed URL. `tabIndex` now reports what was bound. The host attribute still goes to -1 while the link is disabled, so nothing about focus behavior changed. `print` accepts `string | null` instead of `any`, and an unbound link no longer carries the `kbq-link_print` class: the old setter set `printMode = value !== null` and only ran when the input was bound, so the class depended on whether anyone bound `[print]` at all. `disabledSignal` stays a public `WritableSignal` — `kbqTooltip` accepts a link through `forDisabledComponent` and reads it. It is a `linkedSignal` over the input now, so binding still drives it and a direct write still wins. The icon and print bookkeeping — `icons`, `icon`, `hasIcon`, `printMode`, `printUrl` — left the public surface. The `icons` query is a signal query driven by an effect, so an icon projected behind an `@if` gets its spacing class like it does on the badge. BREAKING CHANGE: `KbqLink.disabled`, `tabIndex` and `print` are signal inputs; `tabIndex` reports the bound value rather than -1 for a disabled link, and an unbound link no longer carries `kbq-link_print`. The icon and print bookkeeping is protected or private. Reported and partly rewritten by the `link-signals` schematic. Co-Authored-By: Claude Opus 5 --- docs/guides/migration.en.md | 22 +- docs/guides/migration.ru.md | 22 +- .../components/link/link.component.spec.ts | 83 ++- packages/components/link/link.component.ts | 173 +++---- packages/schematics/src/collection.json | 5 + packages/schematics/src/migrations.json | 5 + .../src/migrations/link-signals/README.md | 61 +++ .../src/migrations/link-signals/data.ts | 89 ++++ .../src/migrations/link-signals/index.spec.ts | 250 +++++++++ .../src/migrations/link-signals/index.ts | 478 ++++++++++++++++++ .../src/migrations/link-signals/schema.json | 20 + .../src/migrations/link-signals/schema.ts | 6 + tools/public_api_guard/components/link.api.md | 54 +- 13 files changed, 1124 insertions(+), 144 deletions(-) create mode 100644 packages/schematics/src/migrations/link-signals/README.md create mode 100644 packages/schematics/src/migrations/link-signals/data.ts create mode 100644 packages/schematics/src/migrations/link-signals/index.spec.ts create mode 100644 packages/schematics/src/migrations/link-signals/index.ts create mode 100644 packages/schematics/src/migrations/link-signals/schema.json create mode 100644 packages/schematics/src/migrations/link-signals/schema.ts diff --git a/docs/guides/migration.en.md b/docs/guides/migration.en.md index 76f2bbaa45..b3d3ca184b 100644 --- a/docs/guides/migration.en.md +++ b/docs/guides/migration.en.md @@ -1035,7 +1035,7 @@ for each option it deselected and reporting the shortened value to the form cont ### 18. Component review (20.3.0) -Ten components went through a full review in 20.3.0: notification-center, popover, search-expandable, select, split-button, title, toast, tooltip, tree and tree-select. Each review closed the members that were never part of the component's contract, moved inputs to signals where that was the point of it, and fixed the behavior it uncovered along the way. Only the changes that reach a consumer are listed here. +Components went through a full review in 20.3.0, in two waves. The first covered notification-center, popover, search-expandable, select, split-button, title, toast, tooltip, tree and tree-select; the second is the one each subsection below belongs to. Each review closed the members that were never part of the component's contract, moved inputs to signals where that was the point of it, and fixed the behavior it uncovered along the way. Only the changes that reach a consumer are listed here. Every schematic named below runs automatically: @@ -1049,6 +1049,26 @@ Most of them report rather than rewrite: what replaces a removed member or a sig ng g @koobiq/components: --project ``` +#### Link + +The three inputs the automated signal migration skipped were all accessors, and each did something beyond storing a value: `disabled` wrote a separate signal, `tabIndex` folded in the disabled state, and `print` was a setter with no getter that also computed the printed URL. + +`disabledSignal` stays a public `WritableSignal` — `kbqTooltip` accepts a link through `forDisabledComponent` and reads it. It is a `linkedSignal` over the `disabled` input now, so binding still drives it and a direct write still wins. + +| Pattern | Manual migration | +| ------------------------------- | -------------------------------------------------------------------------- | +| `.disabled` | Read as `disabled()` — rewritten for you | +| `.tabIndex` | `tabIndex()`, and expect what was bound — not `-1` for a disabled link | +| `.print = …` | Bind `[print]`; it was a setter with no getter, so there is no read to fix | +| `.icons` / `.icon` / `.hasIcon` | Now `protected`/`private`; the icon spacing classes are the contract | +| `.printMode` / `.printUrl` | Now `protected`; the `kbq-link_print` class and `print` attribute are | + +The host attribute still goes to `-1` while the link is disabled, so nothing about focus behavior changed — only a programmatic read of `tabIndex` sees the difference. + +**An unbound link no longer carries the `kbq-link_print` class.** The old setter set `printMode = value !== null` and only ran when the input was bound, so the class depended on whether anyone bound `[print]` at all. It is driven by the input now and is absent until you bind it. `print` accepts `string | null` instead of `any`. + +Handled by `link-signals`: the `disabled` reads are rewritten, the rest is reported. + #### Search expandable Step 4 already renames the `kbq-filter-search` element to `kbq-search-expandable`. That rewrite only ever touched the tag, so the inputs of the removed `KbqFilterBarSearch` survived as attributes the new component does not have — silently, because an unknown attribute on a component is not an error. `v20-upgrade` renames them too now: diff --git a/docs/guides/migration.ru.md b/docs/guides/migration.ru.md index a7d545361d..35b0573e62 100644 --- a/docs/guides/migration.ru.md +++ b/docs/guides/migration.ru.md @@ -1039,7 +1039,7 @@ ng g @koobiq/components:list-tree-multiple-input --project ### 18. Ревью компонентов (20.3.0) -В 20.3.0 полное ревью прошли десять компонентов: notification-center, popover, search-expandable, select, split-button, title, toast, tooltip, tree и tree-select. Каждое ревью закрывало члены, которые никогда не были частью контракта компонента, переводило входы на сигналы там, где в этом и был его смысл, и попутно исправляло найденные ошибки поведения. Ниже перечислено только то, что доходит до потребителя. +В 20.3.0 полное ревью прошло в две волны. В первую вошли notification-center, popover, search-expandable, select, split-button, title, toast, tooltip, tree и tree-select; ко второй относится каждый из подразделов ниже. Каждое ревью закрывало члены, которые никогда не были частью контракта компонента, переводило входы на сигналы там, где в этом и был его смысл, и попутно исправляло найденные ошибки поведения. Ниже перечислено только то, что доходит до потребителя. Все схематики, названные ниже, запускаются автоматически: @@ -1053,6 +1053,26 @@ ng update @koobiq/components@20 ng g @koobiq/components: --project ``` +#### Link + +Три входа, которые автоматическая миграция на сигналы пропустила, были геттерами и сеттерами, и каждый делал больше, чем просто хранил значение: `disabled` писал в отдельный сигнал, `tabIndex` подмешивал состояние блокировки, а `print` был сеттером без геттера, который ещё и вычислял печатаемый URL. + +`disabledSignal` остаётся публичным `WritableSignal` — `kbqTooltip` принимает ссылку через `forDisabledComponent` и читает его. Теперь это `linkedSignal` поверх входа `disabled`, поэтому привязка по-прежнему им управляет, а прямая запись по-прежнему побеждает. + +| Что было | Как мигрировать вручную | +| ------------------------------- | ---------------------------------------------------------------------------------- | +| `.disabled` | Читать как `disabled()` — переписывается за вас | +| `.tabIndex` | `tabIndex()`, и ожидать привязанное значение, а не `-1` для заблокированной ссылки | +| `.print = …` | Привязать `[print]`; это был сеттер без геттера, поэтому чтений нет | +| `.icons` / `.icon` / `.hasIcon` | Стали `protected`/`private`; контракт — классы отступов у иконок | +| `.printMode` / `.printUrl` | Стали `protected`; контракт — класс `kbq-link_print` и атрибут `print` | + +Атрибут на хосте по-прежнему становится `-1`, пока ссылка заблокирована, так что поведение фокуса не изменилось — разницу видит только программное чтение `tabIndex`. + +**Ссылка без привязки `[print]` больше не получает класс `kbq-link_print`.** Старый сеттер выставлял `printMode = value !== null` и запускался только при наличии привязки, поэтому класс зависел от того, привязал ли кто-нибудь `[print]` вообще. Теперь классом управляет сам вход, и до привязки его нет. `print` принимает `string | null` вместо `any`. + +Закрывается схематиком `link-signals`: чтения `disabled` переписываются, остальное сообщается в отчёте. + #### Search expandable Шаг 4 уже переименовывает элемент `kbq-filter-search` в `kbq-search-expandable`. Но та замена трогала только тег, поэтому входы удалённого `KbqFilterBarSearch` оставались в разметке атрибутами, которых у нового компонента нет, — и молча, потому что неизвестный атрибут на компоненте не является ошибкой. Теперь `v20-upgrade` переименовывает и их: diff --git a/packages/components/link/link.component.spec.ts b/packages/components/link/link.component.spec.ts index d160565fa5..66c6981d38 100644 --- a/packages/components/link/link.component.spec.ts +++ b/packages/components/link/link.component.spec.ts @@ -1,7 +1,8 @@ -import { Component } from '@angular/core'; +import { Component, signal } from '@angular/core'; import { fakeAsync, TestBed, tick } from '@angular/core/testing'; +import { By } from '@angular/platform-browser'; import { KbqIconModule } from '@koobiq/components/icon'; -import { KbqLinkModule } from './index'; +import { KbqLink, KbqLinkModule } from './index'; describe('KbqLink', () => { beforeEach(() => { @@ -13,7 +14,8 @@ describe('KbqLink', () => { KbqLinkIconTestApp, KbqLinkPrintTestApp, KbqLinkPseudoTestApp, - KbqLinkNoUnderlineTestApp + KbqLinkNoUnderlineTestApp, + KbqLinkDisabledTestApp ] }).compileComponents(); }); @@ -44,6 +46,9 @@ describe('KbqLink', () => { it('should has .kbq-link_print', fakeAsync(() => { const fixture = TestBed.createComponent(KbqLinkPrintTestApp); + // `print` falls back to the host `href`, which is DOM state: it is read in a microtask after the + // first render, so the attribute lands on the change detection pass that follows. + fixture.detectChanges(); tick(); fixture.detectChanges(); @@ -52,13 +57,22 @@ describe('KbqLink', () => { expect(link.classList).toContain('kbq-link_print'); expect(link.attributes.print.nodeValue).toContain('localhost:3003/'); - fixture.componentInstance.print = 'newUrl'; + fixture.componentInstance.print.set('newUrl'); + fixture.detectChanges(); tick(); + fixture.detectChanges(); + + expect(link.attributes.print.nodeValue).toBe('newUrl'); + })); + + it('should drop .kbq-link_print when print is unbound', fakeAsync(() => { + const fixture = TestBed.createComponent(KbqLinkBaseTestApp); + fixture.detectChanges(); tick(); fixture.detectChanges(); - expect(link.attributes.print.nodeValue).toContain(fixture.componentInstance.print); + expect(fixture.nativeElement.querySelector('[kbq-link]').classList).not.toContain('kbq-link_print'); })); it('should has .kbq-link_pseudo', () => { @@ -80,6 +94,50 @@ describe('KbqLink', () => { expect(link.classList).toContain('kbq-link_no-underline'); }); + it('should take a disabled link out of the tab order', () => { + const fixture = TestBed.createComponent(KbqLinkDisabledTestApp); + + fixture.detectChanges(); + + const link = fixture.nativeElement.querySelector('[kbq-link]'); + + expect(link.attributes.tabIndex.nodeValue).toBe('5'); + expect(link.classList).not.toContain('kbq-disabled'); + + fixture.componentInstance.disabled.set(true); + fixture.detectChanges(); + + expect(link.attributes.tabIndex.nodeValue).toBe('-1'); + expect(link.classList).toContain('kbq-disabled'); + expect(link.getAttribute('disabled')).toBe('true'); + }); + + it('should keep reporting the bound tabIndex while disabled', () => { + const fixture = TestBed.createComponent(KbqLinkDisabledTestApp); + + fixture.detectChanges(); + fixture.componentInstance.disabled.set(true); + fixture.detectChanges(); + + const link = fixture.debugElement.query(By.directive(KbqLink)).injector.get(KbqLink); + + expect(link.tabIndex()).toBe(5); + expect(link.disabled()).toBe(true); + }); + + it('should let a forDisabledComponent consumer drive disabledSignal', () => { + const fixture = TestBed.createComponent(KbqLinkDisabledTestApp); + + fixture.detectChanges(); + + const linkElement = fixture.nativeElement.querySelector('[kbq-link]'); + const link = fixture.debugElement.query(By.directive(KbqLink)).injector.get(KbqLink); + + link.disabledSignal.set(true); + fixture.detectChanges(); + + expect(linkElement.classList).toContain('kbq-disabled'); + }); }); @Component({ @@ -95,11 +153,11 @@ class KbqLinkBaseTestApp {} selector: 'kbq-link-print-test-app', imports: [KbqLinkModule, KbqIconModule], template: ` - Отчет сканирования + Отчет сканирования ` }) class KbqLinkPrintTestApp { - print: string = ''; + readonly print = signal(''); } @Component({ @@ -131,3 +189,14 @@ class KbqLinkPseudoTestApp {} ` }) class KbqLinkNoUnderlineTestApp {} + +@Component({ + selector: 'kbq-link-disabled-test-app', + imports: [KbqLinkModule], + template: ` + Отчет сканирования + ` +}) +class KbqLinkDisabledTestApp { + readonly disabled = signal(false); +} diff --git a/packages/components/link/link.component.ts b/packages/components/link/link.component.ts index 6f1491c375..0bbae2193c 100644 --- a/packages/components/link/link.component.ts +++ b/packages/components/link/link.component.ts @@ -1,24 +1,22 @@ import { FocusMonitor } from '@angular/cdk/a11y'; import { - AfterContentInit, AfterViewInit, booleanAttribute, + computed, contentChild, - ContentChildren, - DestroyRef, + contentChildren, Directive, + effect, ElementRef, forwardRef, inject, - Input, input, + linkedSignal, numberAttribute, OnDestroy, - QueryList, Renderer2, signal } from '@angular/core'; -import { takeUntilDestroyed, toObservable } from '@angular/core/rxjs-interop'; import { getNodesWithoutComments, kbqInjectNativeElement, @@ -27,8 +25,10 @@ import { } from '@koobiq/components/core'; import { KbqIcon } from '@koobiq/components/icon'; +/** @docs-private */ export const baseURLRegex = /^http(s)?:\/\//; +/** Directive that styles an anchor as a link. */ @Directive({ selector: '[kbq-link]', host: { @@ -39,133 +39,126 @@ export const baseURLRegex = /^http(s)?:\/\//; '[class.kbq-link_compact]': 'compact()', '[class.kbq-link_pseudo]': 'pseudo()', '[class.kbq-link_multiline]': 'multiline()', - '[class.kbq-link_print]': 'printMode', - '[class.kbq-text-only]': '!hasIcon', - '[class.kbq-text-with-icon]': 'hasIcon', - '[class.kbq-disabled]': 'disabled', - '[attr.disabled]': 'disabled || null', - '[attr.tabindex]': 'tabIndex', - '[attr.print]': 'printUrl' + '[class.kbq-link_print]': 'printMode()', + '[class.kbq-text-only]': '!hasIcon()', + '[class.kbq-text-with-icon]': 'hasIcon()', + '[class.kbq-disabled]': 'disabledSignal()', + '[attr.disabled]': 'disabledSignal() || null', + '[attr.tabindex]': 'hostTabIndex()', + '[attr.print]': 'printUrl()' }, exportAs: 'kbqLink' }) -export class KbqLink implements AfterContentInit, AfterViewInit, OnDestroy { - private elementRef = inject>(ElementRef); - private focusMonitor = inject(FocusMonitor); +export class KbqLink implements AfterViewInit, OnDestroy { + private readonly focusMonitor = inject(FocusMonitor); + private readonly nativeElement = kbqInjectNativeElement(); protected readonly renderer = inject(Renderer2); - protected readonly destroyRef = inject(DestroyRef); - protected readonly nativeElement = kbqInjectNativeElement(); - @ContentChildren(forwardRef(() => KbqIcon), { read: ElementRef }) icons: QueryList; + private readonly icons = contentChildren( + forwardRef(() => KbqIcon), + { read: ElementRef } + ); - // @todo 20 In the next major release this feature will be replaced on the input signal. /** Whether the link is disabled. */ - // TODO: Skipped for migration because: - // Accessor inputs cannot be migrated as they are too complex. - @Input({ transform: booleanAttribute }) - get disabled(): boolean { - return this._disabled; - } - - set disabled(value: boolean) { - this.disabledSignal.set(value); - } - - /** @docs-private */ - readonly disabledSignal = signal(false); - - // TODO: Skipped for migration because: - // Accessor inputs cannot be migrated as they are too complex. - @Input({ transform: numberAttribute }) - get tabIndex(): number { - return this.disabled ? -1 : this._tabIndex; - } + readonly disabled = input(false, { transform: booleanAttribute }); - set tabIndex(value: number) { - this._tabIndex = value; - } + /** + * Effective disabled state, mirroring the `disabled` input. It stays writable because `kbqTooltip` + * accepts a link through `forDisabledComponent` and reads this signal to keep a tooltip reachable on a + * disabled link; the host bindings read it rather than the input so such a write still shows. + * + * @docs-private + */ + readonly disabledSignal = linkedSignal(() => this.disabled()); - private _tabIndex = 0; + /** Tab order of the link. A disabled link is taken out of the tab order regardless of this value. */ + readonly tabIndex = input(0, { transform: numberAttribute }); - readonly pseudo = input(false, { transform: booleanAttribute }); + /** Whether the link is rendered without a `href`, as a control that looks like a link. */ + readonly pseudo = input(false, { transform: booleanAttribute }); - readonly noUnderline = input(false, { transform: booleanAttribute }); + /** Whether the link is rendered without an underline. */ + readonly noUnderline = input(false, { transform: booleanAttribute }); - readonly big = input(false, { transform: booleanAttribute }); + /** Whether the link uses the big typography. */ + readonly big = input(false, { transform: booleanAttribute }); - readonly compact = input(false, { transform: booleanAttribute }); + /** Whether the link uses the compact typography. */ + readonly compact = input(false, { transform: booleanAttribute }); /** Whether the link has an increased hit area for multiline usage. */ - readonly multiline = input(false, { transform: booleanAttribute }); - - readonly useVisited = input(false, { transform: booleanAttribute }); - - get hasIcon(): boolean { - return !!this.icon(); - } - - // TODO: Skipped for migration because: - // Accessor inputs cannot be migrated as they are too complex. - @Input() - set print(value: any) { - this.printMode = value !== null; + readonly multiline = input(false, { transform: booleanAttribute }); - this._print = value; + /** Whether a visited link is styled differently. */ + readonly useVisited = input(false, { transform: booleanAttribute }); - this.updatePrintUrl(); - } + /** + * URL printed next to the link text when the page is printed. Defaults to the `href` without its + * protocol; bind it to print something else, or bind `null` to opt out. + */ + readonly print = input(); - private _print: string; + /** @docs-private */ + protected readonly icon = contentChild(KbqIcon); - printMode: boolean; + /** @docs-private */ + protected readonly hasIcon = computed(() => !!this.icon()); - printUrl: string; + /** @docs-private */ + protected readonly printMode = computed(() => this.print() != null); - // @todo 20 In the next major release this line will be deleted. - private _disabled: boolean; + /** @docs-private */ + protected readonly hostTabIndex = computed(() => (this.disabledSignal() ? -1 : this.tabIndex())); - readonly icon = contentChild(KbqIcon); + /** @docs-private */ + protected readonly printUrl = signal(undefined); constructor() { - this.updatePrintUrl(); + effect(() => { + const print = this.print(); - // @todo 20 In the next major release this line will be deleted. - toObservable(this.disabledSignal).subscribe((value) => (this._disabled = value)); + // `href` is DOM state rather than a signal, so it is read once the binding that sets it has landed. + Promise.resolve().then(() => + this.printUrl.set(print || this.nativeElement.href?.replace(baseURLRegex, '')) + ); + }); + + // Icons projected asynchronously (e.g. behind an `@if`) update the `icons` signal after content + // init, so class assignment must react to the signal, not just run once. + effect(() => this.updateClassModifierForIcons()); } ngAfterViewInit(): void { - this.focusMonitor.monitor(this.elementRef.nativeElement, true); + this.focusMonitor.monitor(this.nativeElement, true); } - ngOnDestroy() { - this.focusMonitor.stopMonitoring(this.elementRef.nativeElement); + ngOnDestroy(): void { + this.focusMonitor.stopMonitoring(this.nativeElement); } + /** Focuses the link. */ focus(): void { this.getHostElement().focus(); } - getHostElement() { - return this.elementRef.nativeElement; + /** The anchor the directive is applied to. */ + getHostElement(): HTMLAnchorElement { + return this.nativeElement; } - ngAfterContentInit() { - this.updateClassModifierForIcons(); - - this.icons.changes.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(this.updateClassModifierForIcons); - } + private updateClassModifierForIcons(): void { + const icons = this.icons(); - private updateClassModifierForIcons = () => { - this.icons.forEach(({ nativeElement }) => { + icons.forEach(({ nativeElement }) => { this.renderer.removeClass(nativeElement, leftIconClassName); this.renderer.removeClass(nativeElement, rightIconClassName); }); const filteredNodesWithoutComments = getNodesWithoutComments(this.nativeElement.childNodes as NodeList); - if (this.icons.length && filteredNodesWithoutComments.length > 1) { - this.icons.forEach(({ nativeElement }) => { + if (icons.length && filteredNodesWithoutComments.length > 1) { + icons.forEach(({ nativeElement }) => { const iconIndex = filteredNodesWithoutComments.findIndex((node) => node === nativeElement); if (iconIndex === 0) { @@ -177,11 +170,5 @@ export class KbqLink implements AfterContentInit, AfterViewInit, OnDestroy { } }); } - }; - - private updatePrintUrl() { - Promise.resolve().then(() => { - this.printUrl = this._print || this.getHostElement().href?.replace(baseURLRegex, ''); - }); } } diff --git a/packages/schematics/src/collection.json b/packages/schematics/src/collection.json index c2cc44abcb..be175ba4d4 100644 --- a/packages/schematics/src/collection.json +++ b/packages/schematics/src/collection.json @@ -143,6 +143,11 @@ "description": "Reports the KbqSplitButton members whose type changed in the split-button review. disabled reports boolean | undefined instead of boolean: the backing field has no initializer, so a control with no [disabled] binding always returned undefined behind a non-nullable type, and the call sites that assigned it to a boolean or compared it against false were quietly wrong. The protected buttons content query moved from QueryList to a signal query, so a subclass reading buttons.changes or buttons.length has to read buttons() instead. Warn-only: narrowing boolean | undefined back to boolean is a decision the call site owns. Notes the silent part too — an empty no longer throws outside dev mode.", "factory": "./migrations/split-button-optional-disabled/index", "schema": "./migrations/split-button-optional-disabled/schema.json" + }, + "link-signals": { + "description": "Migrates KbqLink consumers to its finished signal-based API. The three inputs the automated signal migration skipped were all accessors that did more than store a value. Rewrites programmatic reads of disabled to calls on receivers typed KbqLink. Warns - without auto-fixing - on tabIndex, which is now a read-only InputSignal reporting what was bound instead of folding in the disabled state, on writes to print, which was a write-only setter input, on the icon and print bookkeeping that left the public surface (icons, icon, hasIcon, printMode, printUrl), and on view/content queries that return the instance. Reports that an unbound link no longer carries the kbq-link_print class, and that the effective disabled state is disabledSignal(), which stays writable for kbqTooltip's forDisabledComponent.", + "factory": "./migrations/link-signals/index", + "schema": "./migrations/link-signals/schema.json" } } } diff --git a/packages/schematics/src/migrations.json b/packages/schematics/src/migrations.json index 152d6b710a..1ac5da0f32 100644 --- a/packages/schematics/src/migrations.json +++ b/packages/schematics/src/migrations.json @@ -105,6 +105,11 @@ "version": "20.3.0-0", "description": "Reports the KbqSplitButton members whose type changed in the split-button review. disabled reports boolean | undefined instead of boolean: the backing field has no initializer, so a control with no [disabled] binding always returned undefined behind a non-nullable type, and the call sites that assigned it to a boolean or compared it against false were quietly wrong. The protected buttons content query moved from QueryList to a signal query, so a subclass reading buttons.changes or buttons.length has to read buttons() instead. Warn-only: narrowing boolean | undefined back to boolean is a decision the call site owns. Notes the silent part too — an empty no longer throws outside dev mode.", "factory": "./migrations/split-button-optional-disabled/index" + }, + "link-signals": { + "version": "20.3.0-0", + "description": "Migrates KbqLink consumers to its finished signal-based API. The three inputs the automated signal migration skipped were all accessors that did more than store a value. Rewrites programmatic reads of disabled to calls on receivers typed KbqLink. Warns - without auto-fixing - on tabIndex, which is now a read-only InputSignal reporting what was bound instead of folding in the disabled state, on writes to print, which was a write-only setter input, on the icon and print bookkeeping that left the public surface (icons, icon, hasIcon, printMode, printUrl), and on view/content queries that return the instance. Reports that an unbound link no longer carries the kbq-link_print class, and that the effective disabled state is disabledSignal(), which stays writable for kbqTooltip's forDisabledComponent.", + "factory": "./migrations/link-signals/index" } } } diff --git a/packages/schematics/src/migrations/link-signals/README.md b/packages/schematics/src/migrations/link-signals/README.md new file mode 100644 index 0000000000..f65d2350aa --- /dev/null +++ b/packages/schematics/src/migrations/link-signals/README.md @@ -0,0 +1,61 @@ +# link-signals + +Migration schematic invoked automatically by `ng update @koobiq/components@20` (registered for +`20.3.0-0`). Migrates `KbqLink` consumers to its finished signal-based API and reports the members +the review closed. + +## Background + +The three inputs the automated signal migration skipped were all accessors, and each did something +beyond storing a value: + +```ts +set disabled(value: boolean) { this.disabledSignal.set(value); } +get tabIndex(): number { return this.disabled ? -1 : this._tabIndex; } +set print(value: any) { this.printMode = value !== null; this._print = value; this.updatePrintUrl(); } +``` + +`disabledSignal` stays a public `WritableSignal`: `kbqTooltip` accepts a link through +`forDisabledComponent` and reads it, and that contract is typed on the tooltip side. It is a +`linkedSignal` over the input now, so binding `[disabled]` still drives it and a direct write still +wins. + +## What it rewrites + +| Before | After | +| --------------- | ----------------- | +| `link.disabled` | `link.disabled()` | + +On receivers explicitly typed `KbqLink`. Already-migrated reads are left alone, so the schematic is +idempotent. There is no template-reference pass: `kbq-link` is an attribute on an anchor, so a +`#ref="kbqLink"` read is not tied to an element name the schematic can match. + +## What it does _not_ do + +| Pattern | Manual migration | +| ------------------------------- | -------------------------------------------------------------------------- | +| `.tabIndex` | `tabIndex()`, and expect what was bound — not `-1` for a disabled link | +| `link.print = …` | Bind `[print]`; it was a setter with no getter, so there is no read to fix | +| `.icons` / `.icon` / `.hasIcon` | Now `protected`/`private`; the icon spacing classes are the contract | +| `.printMode` / `.printUrl` | Now `protected`; the `kbq-link_print` class and `print` attribute are | +| `viewChild(KbqLink)` | The query returns the instance, so a read is a double call | + +`tabIndex` is warned about rather than rewritten because appending `()` would compile and hand back +a different number for a disabled link. + +## Notes with no call site to point at + +- **An unbound link no longer carries `kbq-link_print`.** The old setter set + `printMode = value !== null` and only ran when the input was bound, so the class depended on + whether anyone bound `[print]` at all. It is driven by the input now and is absent until you bind + it. `print` accepts `string | null` instead of `any`. +- **Reading `disabled` reports the bound input.** The effective state — what the host bindings + render — is `disabledSignal()`. The two only differ if something writes `disabledSignal` directly. + +## Running it manually + +``` +ng generate @koobiq/components:link-signals --project my-app +``` + +Pass `--fix=false` to see what it would change without writing. diff --git a/packages/schematics/src/migrations/link-signals/data.ts b/packages/schematics/src/migrations/link-signals/data.ts new file mode 100644 index 0000000000..99406bfab1 --- /dev/null +++ b/packages/schematics/src/migrations/link-signals/data.ts @@ -0,0 +1,89 @@ +/** + * Data for the `link-signals` migration. + * + * The three inputs the automated signal migration skipped were all accessors, and each of them did + * something beyond storing a value. + * + * - `link.disabled` → `link.disabled()` (value unchanged — auto-fixed) + * - `link.tabIndex` → a signal whose value changed: the getter folded in the disabled state (warn) + * - `link.print` → was write-only (a setter with no getter), so it has no read to rewrite (warn on writes) + * - `icons` / `icon` / `hasIcon` / `printMode` / `printUrl` → `protected` / `private` (warn) + * + * `disabledSignal` stays a public `WritableSignal`: `kbqTooltip` accepts a link through + * `forDisabledComponent` and reads it, and that contract is typed on the tooltip side. + */ + +/** Members of `KbqLink` whose value is unchanged; a read must become a call. Auto-fixed. */ +export const SIGNAL_MEMBERS: readonly string[] = ['disabled']; + +/** + * Signal members that are writable via `.set(...)`. Every `KbqLink` signal member is `input()` + * (read-only), so this is empty — a programmatic write is left untouched and becomes a compile error. + */ +export const WRITABLE_MEMBERS: ReadonlySet = new Set(); + +/** TypeScript type annotation that marks a receiver as a link. */ +export const LINK_TYPE = 'KbqLink'; + +/** + * Attribute selector whose template reference variables (`#ref="kbqLink"`) point at a link. The link is a + * directive rather than an element, so there is no element name to match — template reference rewriting + * only applies to ``-shaped hosts, which do not exist. Kept for the shared machinery. + */ +export const LINK_ELEMENT = 'kbq-link'; + +/** Import specifier that marks a file as a link consumer. */ +export const LINK_PACKAGE = '@koobiq/components/link'; + +/** + * `tabIndex` became a read-only `InputSignal` AND changed its value: the getter returned `-1` for a + * disabled link, it now reports what was bound. The host attribute still goes to `-1` while the link is + * disabled, so nothing about focus behavior changed — only a programmatic read sees the difference, and a + * mechanical `()` append would silently change it. + */ +export const VALUE_CHANGED_MEMBERS: readonly string[] = ['tabIndex']; + +/** Members that moved out of the public surface and can no longer be read from outside the directive. */ +export const PROTECTED_MEMBERS: readonly string[] = ['icons', 'icon', 'hasIcon', 'printMode', 'printUrl']; + +/** Appended to the protected-members warning. */ +export const PROTECTED_HINT = + 'They are the icon-spacing and print bookkeeping: the classes and the `print` attribute the directive ' + + 'puts on the anchor are the contract, not the state behind them.'; + +export interface WarnPattern { + /** Owner of the member. The pattern is only evaluated for files that also name it. */ + anchor: string; + pattern: string; + message: string; +} + +const LINK_ANCHOR = '\\bKbqLink\\b'; + +export const warnPatterns: WarnPattern[] = [ + { + anchor: LINK_ANCHOR, + pattern: '\\.\\s*print\\s*=', + message: + 'KbqLink.print was a write-only input (a setter with no getter) and is an `input()` now, so the ' + + 'write no longer compiles. Bind `[print]` in the template instead. Binding `null` opts out of the ' + + 'printed URL, which is what an unbound link does by default.' + }, + { + anchor: LINK_ANCHOR, + pattern: '(?:viewChild|ViewChild|contentChild|ContentChild)[^\\n;]*\\bKbqLink\\b', + message: + 'A KbqLink view/content query returns the directive instance, whose `disabled` is now a signal — ' + + 'reading it is a double call, e.g. `this.link().disabled()`. Verify query reads manually.' + } +]; + +/** Printed once per project, after the per-file reports. */ +export const SUMMARY = [ + ' `print` accepts `string | null` instead of `any`, and an unbound link no longer carries the ' + + '`kbq-link_print` class. The old setter set `printMode = value !== null`, so it only ever ran when ' + + 'the input was bound; the class is now driven by the input itself and is absent until you bind it.', + ' Reading `disabled` reports the bound input. The effective state — what the host bindings render — is ' + + "`disabledSignal()`, which stays writable so `kbqTooltip`'s `forDisabledComponent` keeps working. " + + 'The two only differ if something writes `disabledSignal` directly.' +]; diff --git a/packages/schematics/src/migrations/link-signals/index.spec.ts b/packages/schematics/src/migrations/link-signals/index.spec.ts new file mode 100644 index 0000000000..283123e6e3 --- /dev/null +++ b/packages/schematics/src/migrations/link-signals/index.spec.ts @@ -0,0 +1,250 @@ +import { workspaces } from '@angular-devkit/core'; +import { Tree } from '@angular-devkit/schematics'; +import { SchematicTestRunner } from '@angular-devkit/schematics/testing'; +import { getWorkspace } from '@schematics/angular/utility/workspace'; +import * as path from 'path'; +import { createTestApp } from '../../utils/testing'; +import { Schema } from './schema'; + +const collectionPath = path.join(__dirname, '../../collection.json'); +const SCHEMATIC_NAME = 'link-signals'; + +describe(SCHEMATIC_NAME, () => { + let runner: SchematicTestRunner; + let appTree: Tree; + let projects: workspaces.ProjectDefinitionCollection; + let messages: string[]; + + beforeEach(async () => { + runner = new SchematicTestRunner('schematics', collectionPath); + appTree = await createTestApp(runner, { style: 'scss' }); + const workspace = await getWorkspace(appTree); + + projects = workspace.projects as unknown as workspaces.ProjectDefinitionCollection; + + messages = []; + runner.logger.subscribe((entry) => messages.push(entry.message)); + }); + + function paths(project: workspaces.ProjectDefinition) { + const root = `/${project.root}/src/app`; + const ts = appTree.exists(`${root}/app.ts`) ? `${root}/app.ts` : `${root}/app.component.ts`; + const html = appTree.exists(`${root}/app.html`) ? `${root}/app.html` : `${root}/app.component.html`; + + return { ts, html }; + } + + async function run(fix: boolean = true): Promise { + const [first] = projects.keys(); + + return runner.runSchematic(SCHEMATIC_NAME, { project: first, fix } satisfies Schema, appTree); + } + + function firstTsPath(): string { + const [first] = projects.keys(); + + return paths(projects.get(first)!).ts; + } + + it('rewrites disabled reads on a parameter typed KbqLink (incl. optional chain) to calls', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { KbqLink } from '@koobiq/components/link';\n" + + 'class Demo {\n' + + ' read(link: KbqLink) {\n' + + ' return link.disabled ?? link?.disabled;\n' + + ' }\n' + + '}\n' + ); + + const updated = (await run()).readText(ts); + + expect(updated).toContain('link.disabled() ?? link?.disabled()'); + }); + + it('rewrites reads on a @ViewChild field (this.link)', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { ViewChild } from '@angular/core';\n" + + "import { KbqLink } from '@koobiq/components/link';\n" + + 'class Demo {\n' + + ' @ViewChild(KbqLink) link: KbqLink;\n' + + ' read() {\n' + + ' return this.link.disabled;\n' + + ' }\n' + + '}\n' + ); + + expect((await run()).readText(ts)).toContain('return this.link.disabled();'); + }); + + it('leaves reads on a receiver of an unrelated type alone', async () => { + const ts = firstTsPath(); + const source = + "import { KbqLink } from '@koobiq/components/link';\n" + + 'class Other {\n' + + ' disabled = false;\n' + + '}\n' + + 'class Demo {\n' + + ' read(other: Other) {\n' + + ' return other.disabled;\n' + + ' }\n' + + '}\n'; + + appTree.overwrite(ts, source); + + expect((await run()).readText(ts)).toBe(source); + }); + + it('is idempotent — an already migrated read is left alone', async () => { + const ts = firstTsPath(); + const source = + "import { KbqLink } from '@koobiq/components/link';\n" + + 'class Demo {\n' + + ' read(link: KbqLink) {\n' + + ' return link.disabled();\n' + + ' }\n' + + '}\n'; + + appTree.overwrite(ts, source); + + expect((await run()).readText(ts)).toBe(source); + }); + + it('leaves a programmatic write alone — the input is read-only', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { KbqLink } from '@koobiq/components/link';\n" + + 'class Demo {\n' + + ' write(link: KbqLink) {\n' + + ' link.disabled = true;\n' + + ' }\n' + + '}\n' + ); + + expect((await run()).readText(ts)).toContain('link.disabled = true;'); + }); + + it('warns about the icon and print bookkeeping that left the public surface', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { KbqLink } from '@koobiq/components/link';\n" + + 'class Demo {\n' + + ' read(link: KbqLink) {\n' + + ' return link.hasIcon + link.printMode + link.printUrl;\n' + + ' }\n' + + '}\n' + ); + + await run(); + + const logged = messages.join('\n'); + + expect(logged).toContain('hasIcon'); + expect(logged).toContain('printMode'); + expect(logged).toContain('printUrl'); + }); + + it('warns about a view query returning the instance', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { viewChild } from '@angular/core';\n" + + "import { KbqLink } from '@koobiq/components/link';\n" + + 'class Demo {\n' + + ' readonly link = viewChild(KbqLink);\n' + + '}\n' + ); + + await run(); + + expect(messages.join('\n')).toContain('double call'); + }); + + it('warns about tabIndex instead of rewriting it', async () => { + const ts = firstTsPath(); + const source = + "import { KbqLink } from '@koobiq/components/link';\n" + + 'class Demo {\n' + + ' read(link: KbqLink) {\n' + + ' return link.tabIndex;\n' + + ' }\n' + + '}\n'; + + appTree.overwrite(ts, source); + + expect((await run()).readText(ts)).toContain('return link.tabIndex;'); + expect(messages.join('\n')).toContain('disabled state'); + }); + + it('warns about a write to the write-only print input', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { KbqLink } from '@koobiq/components/link';\n" + + 'class Demo {\n' + + ' write(link: KbqLink) {\n' + + " link.print = 'https://koobiq.io';\n" + + ' }\n' + + '}\n' + ); + + await run(); + + expect(messages.join('\n')).toContain('write-only input'); + }); + + it('reports the print and disabledSignal notes once per project', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { KbqLink } from '@koobiq/components/link';\n" + + 'class Demo {\n' + + ' read(link: KbqLink) {\n' + + ' return link.disabled;\n' + + ' }\n' + + '}\n' + ); + + await run(); + + const summary = messages.join('\n'); + + expect(summary).toContain('kbq-link_print'); + expect(summary).toContain('disabledSignal()'); + expect(summary.match(/kbq-link_print/g)!.length).toBe(1); + }); + + it('stays silent for a workspace that does not use the link', async () => { + await run(); + + expect(messages.join('\n')).not.toContain(`[${SCHEMATIC_NAME}]`); + }); + + it('does not write when fix is false', async () => { + const ts = firstTsPath(); + const source = + "import { KbqLink } from '@koobiq/components/link';\n" + + 'class Demo {\n' + + ' read(link: KbqLink) {\n' + + ' return link.disabled;\n' + + ' }\n' + + '}\n'; + + appTree.overwrite(ts, source); + + expect((await run(false)).readText(ts)).toBe(source); + expect(messages.join('\n')).toContain('would update'); + }); +}); diff --git a/packages/schematics/src/migrations/link-signals/index.ts b/packages/schematics/src/migrations/link-signals/index.ts new file mode 100644 index 0000000000..359407eeeb --- /dev/null +++ b/packages/schematics/src/migrations/link-signals/index.ts @@ -0,0 +1,478 @@ +import { Path } from '@angular-devkit/core'; +import { Rule, SchematicContext, Tree } from '@angular-devkit/schematics'; +import ts from 'typescript'; +import { visitAll, Visitor } from '../../utils/ast'; +import { logMessage } from '../../utils/messages'; +import { setupOptions } from '../../utils/package-config'; +import { forEachClass, parseTemplate } from '../../utils/typescript'; +import { + LINK_ELEMENT, + LINK_PACKAGE, + LINK_TYPE, + PROTECTED_HINT, + PROTECTED_MEMBERS, + SIGNAL_MEMBERS, + SUMMARY, + VALUE_CHANGED_MEMBERS, + warnPatterns, + WRITABLE_MEMBERS +} from './data'; +import { Schema } from './schema'; + +const LABEL = '[link-signals]'; +const TS_EXT = '.ts'; +const HTML_EXT = '.html'; + +/** A text-span edit on the original file content. Applied right-to-left so offsets stay valid. */ +interface Edit { + start: number; + end: number; + text: string; +} + +/** A receiver whose static type is a link, valid within `[start, end]` of the source. */ +interface Receiver { + /** Source text of the receiver expression, e.g. `link` or `this.link`. */ + text: string; + start: number; + end: number; +} + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +/** Applies text-span edits to `content`, right-to-left, so earlier edits don't shift later offsets. */ +function applyEdits(content: string, edits: Edit[]): string { + const sorted = [...edits].sort((a, b) => b.start - a.start || b.end - a.end); + let result = content; + + for (const { start, end, text } of sorted) { + result = result.slice(0, start) + text + result.slice(end); + } + + return result; +} + +const isFunctionLike = (node: ts.Node): boolean => + ts.isFunctionDeclaration(node) || + ts.isMethodDeclaration(node) || + ts.isConstructorDeclaration(node) || + ts.isArrowFunction(node) || + ts.isFunctionExpression(node) || + ts.isGetAccessorDeclaration(node) || + ts.isSetAccessorDeclaration(node); + +/** Walks up from `node` to the nearest ancestor matching `predicate`. */ +function findAncestor(node: ts.Node, predicate: (node: ts.Node) => boolean): ts.Node | undefined { + let current = node.parent; + + while (current) { + if (predicate(current)) return current; + current = current.parent; + } + + return undefined; +} + +/** Whether a type annotation refers to `typeName`. */ +function isTypeReference(type: ts.TypeNode | undefined, typeName: string): boolean { + return !!type && ts.isTypeReferenceNode(type) && ts.isIdentifier(type.typeName) && type.typeName.text === typeName; +} + +const FIELD_MODIFIERS = new Set([ + ts.SyntaxKind.PrivateKeyword, + ts.SyntaxKind.PublicKeyword, + ts.SyntaxKind.ProtectedKeyword, + ts.SyntaxKind.ReadonlyKeyword +]); + +/** + * Collects the receivers annotated with `typeName`, by explicit annotation only (no cross-package type + * resolution): method/function params, class fields (incl. `@ViewChild(KbqLink) x: KbqLink` and constructor + * parameter-properties) and typed locals. + */ +function collectReceivers(sourceFile: ts.SourceFile, typeName: string): Receiver[] { + const receivers: Receiver[] = []; + const add = (text: string, scope: ts.Node) => + receivers.push({ text, start: scope.getStart(sourceFile), end: scope.getEnd() }); + + const visit = (node: ts.Node): void => { + if (ts.isParameter(node) && ts.isIdentifier(node.name) && isTypeReference(node.type, typeName)) { + add(node.name.text, findAncestor(node, isFunctionLike) ?? sourceFile); + + // A constructor parameter-property is also a class field, reachable as `this.`. + if (node.modifiers?.some((modifier) => FIELD_MODIFIERS.has(modifier.kind))) { + const owner = findAncestor(node, ts.isClassDeclaration); + + if (owner) add(`this.${node.name.text}`, owner); + } + } else if ( + ts.isPropertyDeclaration(node) && + ts.isIdentifier(node.name) && + isTypeReference(node.type, typeName) + ) { + const owner = findAncestor(node, ts.isClassDeclaration); + + if (owner) add(`this.${node.name.text}`, owner); + } else if ( + ts.isVariableDeclaration(node) && + ts.isIdentifier(node.name) && + isTypeReference(node.type, typeName) + ) { + add(node.name.text, findAncestor(node, isFunctionLike) ?? sourceFile); + } + + node.forEachChild(visit); + }; + + visit(sourceFile); + + return receivers; +} + +/** Whether a property access on a receiver is within one of the receiver's scopes. */ +function inReceiverScope(node: ts.PropertyAccessExpression, sourceFile: ts.SourceFile, receivers: Receiver[]): boolean { + const receiverText = node.expression.getText(sourceFile); + const start = node.getStart(sourceFile); + const end = node.getEnd(); + + return receivers.some((r) => r.text === receiverText && start >= r.start && end <= r.end); +} + +/** Classifies a matched property access and appends the resulting edit(s). */ +function classifyAccess(node: ts.PropertyAccessExpression, sourceFile: ts.SourceFile, edits: Edit[]): void { + const parent = node.parent; + + // Already migrated: a call, or a `.set(...)` write — leave alone (idempotent). + if (ts.isCallExpression(parent) && parent.expression === node) return; + if (ts.isPropertyAccessExpression(parent) && parent.expression === node && parent.name.text === 'set') return; + + // Write target: `x.member = RHS`. Every KbqLink signal member is `input()` (read-only), so there is no + // writable member — leave the write untouched (it becomes a compile error the consumer fixes by hand). + if ( + ts.isBinaryExpression(parent) && + parent.left === node && + parent.operatorToken.kind === ts.SyntaxKind.EqualsToken + ) { + if (WRITABLE_MEMBERS.has(node.name.text)) { + const rhs = parent.right; + + edits.push({ start: node.getEnd(), end: rhs.getStart(sourceFile), text: '.set(' }); + edits.push({ start: rhs.getEnd(), end: rhs.getEnd(), text: ')' }); + } + + return; + } + + // Read (incl. optional chain `x?.compact`): append `()`. + edits.push({ start: node.getEnd(), end: node.getEnd(), text: '()' }); +} + +/** Collects edits for every read/write of a value-safe signal member on a known link receiver. */ +function collectAccessEdits(sourceFile: ts.SourceFile, receivers: Receiver[]): Edit[] { + const edits: Edit[] = []; + + const visit = (node: ts.Node): void => { + if ( + ts.isPropertyAccessExpression(node) && + ts.isIdentifier(node.name) && + SIGNAL_MEMBERS.includes(node.name.text) && + inReceiverScope(node, sourceFile, receivers) + ) { + classifyAccess(node, sourceFile, edits); + } + + node.forEachChild(visit); + }; + + visit(sourceFile); + + return edits; +} + +/** Distinct member names accessed on a link receiver that need manual migration. */ +interface ReceiverWarnings { + protectedAccess: Set; + valueChanged: Set; +} + +/** Collects the members read on a link receiver that no consumer can keep reading as-is. */ +function collectReceiverWarnings(sourceFile: ts.SourceFile, receivers: Receiver[]): ReceiverWarnings { + const protectedAccess = new Set(); + const valueChanged = new Set(); + + const visit = (node: ts.Node): void => { + if ( + ts.isPropertyAccessExpression(node) && + ts.isIdentifier(node.name) && + inReceiverScope(node, sourceFile, receivers) + ) { + const name = node.name.text; + + if (PROTECTED_MEMBERS.includes(name)) protectedAccess.add(name); + else if (VALUE_CHANGED_MEMBERS.includes(name)) valueChanged.add(name); + } + + node.forEachChild(visit); + }; + + visit(sourceFile); + + return { protectedAccess, valueChanged }; +} + +/** Pass A — rewrite value-safe programmatic reads of link signal members in TypeScript code. */ +function migrateTsExpressions(content: string, fileName: string): string { + const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS); + const receivers = collectReceivers(sourceFile, LINK_TYPE); + + if (receivers.length === 0) return content; + + const edits = collectAccessEdits(sourceFile, receivers); + + return edits.length > 0 ? applyEdits(content, edits) : content; +} + +/** Emits precise, receiver-scoped warnings for the members that can't be auto-fixed. */ +function warnReceiverMembers(context: SchematicContext, filePath: string, content: string): void { + const sourceFile = ts.createSourceFile(filePath, content, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS); + const receivers = collectReceivers(sourceFile, LINK_TYPE); + + if (receivers.length === 0) return; + + const { protectedAccess, valueChanged } = collectReceiverWarnings(sourceFile, receivers); + + if (valueChanged.size > 0) { + logMessage(context.logger, [ + `${LABEL} ${filePath}`, + ` \`tabIndex\` is a read-only InputSignal — read it as \`link.tabIndex()\`. Its value also changed:`, + ` the getter used to fold in the disabled state and report -1 for a disabled link, it now reports`, + ` what was bound. The host attribute still goes to -1 while the link is disabled. Migrate by hand.` + ]); + } + + if (protectedAccess.size > 0) { + logMessage(context.logger, [ + `${LABEL} ${filePath}`, + ` These KbqLink members are now \`protected\` and can't be read from outside the ` + + `component: ${[...protectedAccess].join(', ')}. ${PROTECTED_HINT}` + ]); + } +} + +/** Collects template reference variable names bound to a `` element. */ +class LinkRefCollector implements Visitor { + readonly refs = new Set(); + + visitElement(element: any): void { + if (element.name === LINK_ELEMENT) { + for (const attr of element.attrs ?? []) { + if (typeof attr.name !== 'string') continue; + + if (attr.name.startsWith('#')) this.refs.add(attr.name.slice(1)); + else if (attr.name.startsWith('ref-')) this.refs.add(attr.name.slice(4)); + } + } + + this.visitChildren(element); + } + + visitBlock(block: any): void { + this.visitChildren(block); + } + + private visitChildren(node: any): void { + for (const child of node.children ?? []) { + child.visit(this); + } + } + + visitAttribute(): void {} + visitText(): void {} + visitComment(): void {} + visitExpansion(): void {} + visitExpansionCase(): void {} + visitBlockParameter(): void {} + visitLetDeclaration(): void {} +} + +/** Rewrites `ref.member` reads to `ref.member()` for the given refs, scoped to those exact identifiers. */ +function rewriteRefReads(template: string, refs: string[]): { content: string; changed: boolean } { + const members = SIGNAL_MEMBERS.join('|'); + let content = template; + let changed = false; + + for (const ref of refs) { + // `\bref\.(member)\b(?!\s*\()` — skip anything already invoked, so the rewrite is idempotent. + const pattern = new RegExp(`\\b(${escapeRegExp(ref)})\\.(${members})\\b(?!\\s*\\()`, 'g'); + const next = content.replace(pattern, '$1.$2()'); + + if (next !== content) { + content = next; + changed = true; + } + } + + return { content, changed }; +} + +/** Pass B (core) — parse a template, discover link refs, rewrite their value-safe signal reads. */ +async function migrateTemplate(template: string): Promise<{ content: string; changed: boolean }> { + if (!template.includes(LINK_ELEMENT)) return { content: template, changed: false }; + + const parsed = await parseTemplate(template); + + if (!parsed.tree) return { content: template, changed: false }; + + const collector = new LinkRefCollector(); + + visitAll(collector, (parsed.tree as { rootNodes: unknown[] }).rootNodes); + + if (collector.refs.size === 0) return { content: template, changed: false }; + + return rewriteRefReads(template, [...collector.refs]); +} + +/** Interior `[start, end]` ranges of inline `@Component({ template: '…' })` string literals. */ +function collectInlineTemplateRanges(sourceFile: ts.SourceFile): Array<{ start: number; end: number }> { + const ranges: Array<{ start: number; end: number }> = []; + + forEachClass(sourceFile, (node) => { + const decorator = ts + .getDecorators(node) + ?.find( + (dec) => + ts.isCallExpression(dec.expression) && + ts.isIdentifier(dec.expression.expression) && + dec.expression.expression.text === 'Component' + ); + + if (!decorator || !ts.isCallExpression(decorator.expression)) return; + + const [arg] = decorator.expression.arguments; + + if (!arg || !ts.isObjectLiteralExpression(arg)) return; + + for (const prop of arg.properties) { + if ( + ts.isPropertyAssignment(prop) && + (ts.isIdentifier(prop.name) || ts.isStringLiteralLike(prop.name)) && + prop.name.text === 'template' && + ts.isStringLiteralLike(prop.initializer) && + prop.initializer.text + ) { + // +1 / -1 to exclude the opening/closing quote characters. + ranges.push({ start: prop.initializer.getStart(sourceFile) + 1, end: prop.initializer.getEnd() - 1 }); + } + } + }); + + return ranges; +} + +/** Pass B (inline) — rewrite link ref reads inside inline component templates. */ +async function migrateInlineTemplates(content: string, fileName: string): Promise { + const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS); + const ranges = collectInlineTemplateRanges(sourceFile).sort((a, b) => b.start - a.start); + let result = content; + + for (const { start, end } of ranges) { + const { content: rewritten, changed } = await migrateTemplate(result.slice(start, end)); + + if (changed) { + result = result.slice(0, start) + rewritten + result.slice(end); + } + } + + return result; +} + +function logWarnings(context: SchematicContext, filePath: string, content: string): void { + for (const { anchor, pattern, message } of warnPatterns) { + if (!new RegExp(anchor).test(content) || !new RegExp(pattern).test(content)) continue; + + logMessage(context.logger, [`${LABEL} ${filePath}`, ` ${message}`]); + } +} + +/** + * A `.ts` file is a link consumer if it names any of the exported symbols, imports the package, or renders + * the element in an inline template — a component that only imports `KbqLinkModule` names no type. + */ +function referencesLink(content: string): boolean { + return /\bKbqLink\w*\b/.test(content) || content.includes(LINK_PACKAGE) || content.includes(`<${LINK_ELEMENT}`); +} + +export default function linkSignals(options: Schema): Rule { + return async (tree: Tree, context: SchematicContext) => { + const { project, fix } = options; + const projectDefinition = await setupOptions(project, tree); + const root = projectDefinition?.root ?? ''; + const rootDir = root ? tree.getDir(root as Path) : tree.root; + + const tsPaths: string[] = []; + const htmlPaths: string[] = []; + + rootDir.visit((filePath) => { + if (filePath.includes('node_modules') || filePath.includes('/dist/')) return; + + if (filePath.endsWith(TS_EXT)) tsPaths.push(filePath); + else if (filePath.endsWith(HTML_EXT)) htmlPaths.push(filePath); + }); + + let touched = 0; + let consumers = 0; + + const commit = (filePath: string, original: string, updated: string) => { + if (updated === original) return; + + touched++; + + if (fix) { + tree.overwrite(filePath, updated); + } else { + logMessage(context.logger, [`${LABEL} would update ${filePath} (run with --fix to apply)`]); + } + }; + + for (const filePath of tsPaths) { + const original = tree.read(filePath)?.toString(); + + if (!original || !referencesLink(original)) continue; + + consumers++; + + logWarnings(context, filePath, original); + warnReceiverMembers(context, filePath, original); + + let content = migrateTsExpressions(original, filePath); + + content = await migrateInlineTemplates(content, filePath); + + commit(filePath, original, content); + } + + for (const filePath of htmlPaths) { + const original = tree.read(filePath)?.toString(); + + if (!original) continue; + + const { content, changed } = await migrateTemplate(original); + + if (changed) { + consumers++; + commit(filePath, original, content); + } + } + + // Nothing here uses the link, so the summary would only be noise. + if (consumers === 0) return; + + logMessage(context.logger, [ + `${LABEL} processed tree under "${root || ''}", ` + + `${fix ? 'updated' : 'would update'} ${touched} file(s).`, + ...SUMMARY + ]); + }; +} diff --git a/packages/schematics/src/migrations/link-signals/schema.json b/packages/schematics/src/migrations/link-signals/schema.json new file mode 100644 index 0000000000..caa755c172 --- /dev/null +++ b/packages/schematics/src/migrations/link-signals/schema.json @@ -0,0 +1,20 @@ +{ + "$schema": "http://json-schema.org/schema", + "$id": "koobiq-components-link-signals", + "title": "Koobiq components link signals migration", + "type": "object", + "properties": { + "project": { + "type": "string", + "description": "Name of the project to migrate. If omitted, the migration runs over the whole tree.", + "$default": { + "$source": "projectName" + } + }, + "fix": { + "type": "boolean", + "default": true, + "description": "When true, applies all auto-fix replacements. When false, prints what would change without writing." + } + } +} diff --git a/packages/schematics/src/migrations/link-signals/schema.ts b/packages/schematics/src/migrations/link-signals/schema.ts new file mode 100644 index 0000000000..b9f56acac5 --- /dev/null +++ b/packages/schematics/src/migrations/link-signals/schema.ts @@ -0,0 +1,6 @@ +export interface Schema { + /** Name of the project to migrate. */ + project?: string; + /** When true, applies replacements; when false, only logs what would change. */ + fix: boolean; +} diff --git a/tools/public_api_guard/components/link.api.md b/tools/public_api_guard/components/link.api.md index 71234e3d46..5bf38b7164 100644 --- a/tools/public_api_guard/components/link.api.md +++ b/tools/public_api_guard/components/link.api.md @@ -4,74 +4,44 @@ ```ts -import { AfterContentInit } from '@angular/core'; import { AfterViewInit } from '@angular/core'; import * as _angular_core from '@angular/core'; -import { DestroyRef } from '@angular/core'; -import { ElementRef } from '@angular/core'; import * as i1 from '@angular/cdk/a11y'; import { KbqIcon } from '@koobiq/components/icon'; import { OnDestroy } from '@angular/core'; -import { QueryList } from '@angular/core'; import { Renderer2 } from '@angular/core'; -// @public (undocumented) +// @public export const baseURLRegex: RegExp; -// @public (undocumented) -export class KbqLink implements AfterContentInit, AfterViewInit, OnDestroy { +// @public +export class KbqLink implements AfterViewInit, OnDestroy { constructor(); - // (undocumented) readonly big: _angular_core.InputSignalWithTransform; - // (undocumented) readonly compact: _angular_core.InputSignalWithTransform; - // (undocumented) - protected readonly destroyRef: DestroyRef; - get disabled(): boolean; - set disabled(value: boolean); + readonly disabled: _angular_core.InputSignalWithTransform; readonly disabledSignal: _angular_core.WritableSignal; - // (undocumented) focus(): void; - // (undocumented) getHostElement(): HTMLAnchorElement; - // (undocumented) - get hasIcon(): boolean; - // (undocumented) - readonly icon: _angular_core.Signal; - // (undocumented) - icons: QueryList; + protected readonly hasIcon: _angular_core.Signal; + protected readonly hostTabIndex: _angular_core.Signal; + protected readonly icon: _angular_core.Signal; readonly multiline: _angular_core.InputSignalWithTransform; // (undocumented) - protected readonly nativeElement: HTMLElement; - // (undocumented) - static ngAcceptInputType_disabled: unknown; - // (undocumented) - static ngAcceptInputType_tabIndex: unknown; - // (undocumented) - ngAfterContentInit(): void; - // (undocumented) ngAfterViewInit(): void; // (undocumented) ngOnDestroy(): void; - // (undocumented) readonly noUnderline: _angular_core.InputSignalWithTransform; - // (undocumented) - set print(value: any); - // (undocumented) - printMode: boolean; - // (undocumented) - printUrl: string; - // (undocumented) + readonly print: _angular_core.InputSignal; + protected readonly printMode: _angular_core.Signal; + protected readonly printUrl: _angular_core.WritableSignal; readonly pseudo: _angular_core.InputSignalWithTransform; // (undocumented) protected readonly renderer: Renderer2; - // (undocumented) - get tabIndex(): number; - set tabIndex(value: number); - // (undocumented) + readonly tabIndex: _angular_core.InputSignalWithTransform; readonly useVisited: _angular_core.InputSignalWithTransform; // (undocumented) - static ɵdir: _angular_core.ɵɵDirectiveDeclaration; + static ɵdir: _angular_core.ɵɵDirectiveDeclaration; // (undocumented) static ɵfac: _angular_core.ɵɵFactoryDeclaration; } From 65d38acf9c09fcc5cc597fc157e85830b415c140 Mon Sep 17 00:00:00 2001 From: Artem Belik Date: Thu, 3 Sep 2026 17:07:03 +0300 Subject: [PATCH 2/2] fix(link): address the review of the component review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guide announced a behavior change that does not happen. `printMode` had no initializer, so an unbound link left it `undefined` and got no class — exactly as it does now. The real difference is `[print]="undefined"`: `value !== null` passed it and marked the link printable, `!= null` does not. Corrected everywhere and pinned by a test, along with the unbound case still writing the href into the `print` attribute. `contentChild(KbqIcon)` and `contentChildren(KbqIcon, { read: ElementRef })` ran side by side with the same predicate; `hasIcon` is `icons().length > 0`. The print-URL microtask was never cancelled, so a superseded value could still be written after the effect had moved on. The schematic's whole template pass was dead — `kbq-link` is an attribute on an anchor, so `element.name === 'kbq-link'` never holds — while still parsing every consumer's HTML. It also said nothing about `nativeElement` going private or `destroyRef` being removed, both of which a subclass could see. Co-Authored-By: Claude Opus 5 --- docs/guides/migration.en.md | 2 +- docs/guides/migration.ru.md | 2 +- .../components/link/link.component.spec.ts | 17 +- packages/components/link/link.component.ts | 23 +-- .../src/migrations/link-signals/README.md | 9 +- .../src/migrations/link-signals/data.ts | 28 +-- .../src/migrations/link-signals/index.ts | 162 +----------------- tools/public_api_guard/components/link.api.md | 4 +- 8 files changed, 57 insertions(+), 190 deletions(-) diff --git a/docs/guides/migration.en.md b/docs/guides/migration.en.md index b3d3ca184b..76835d46dc 100644 --- a/docs/guides/migration.en.md +++ b/docs/guides/migration.en.md @@ -1065,7 +1065,7 @@ The three inputs the automated signal migration skipped were all accessors, and The host attribute still goes to `-1` while the link is disabled, so nothing about focus behavior changed — only a programmatic read of `tabIndex` sees the difference. -**An unbound link no longer carries the `kbq-link_print` class.** The old setter set `printMode = value !== null` and only ran when the input was bound, so the class depended on whether anyone bound `[print]` at all. It is driven by the input now and is absent until you bind it. `print` accepts `string | null` instead of `any`. +**`[print]="undefined"` no longer marks the link as printable.** The old setter tested `value !== null`, so an explicit `undefined` passed it: the link got `kbq-link_print` and printed its `href`. The input tests `!= null`, which covers both. An unbound link behaves exactly as before — no class, and the href still lands in the `print` attribute. `print` accepts `string | null` instead of `any`. Handled by `link-signals`: the `disabled` reads are rewritten, the rest is reported. diff --git a/docs/guides/migration.ru.md b/docs/guides/migration.ru.md index 35b0573e62..ce51b983ac 100644 --- a/docs/guides/migration.ru.md +++ b/docs/guides/migration.ru.md @@ -1069,7 +1069,7 @@ ng g @koobiq/components: --project Атрибут на хосте по-прежнему становится `-1`, пока ссылка заблокирована, так что поведение фокуса не изменилось — разницу видит только программное чтение `tabIndex`. -**Ссылка без привязки `[print]` больше не получает класс `kbq-link_print`.** Старый сеттер выставлял `printMode = value !== null` и запускался только при наличии привязки, поэтому класс зависел от того, привязал ли кто-нибудь `[print]` вообще. Теперь классом управляет сам вход, и до привязки его нет. `print` принимает `string | null` вместо `any`. +**`[print]="undefined"` больше не помечает ссылку как печатаемую.** Старый сеттер проверял `value !== null`, поэтому явный `undefined` эту проверку проходил: ссылка получала класс `kbq-link_print` и печатала свой `href`. Вход проверяет `!= null`, что покрывает оба случая. Ссылка без привязки ведёт себя ровно как раньше: класса нет, а `href` по-прежнему попадает в атрибут `print`. `print` принимает `string | null` вместо `any`. Закрывается схематиком `link-signals`: чтения `disabled` переписываются, остальное сообщается в отчёте. diff --git a/packages/components/link/link.component.spec.ts b/packages/components/link/link.component.spec.ts index 66c6981d38..4b8e2723da 100644 --- a/packages/components/link/link.component.spec.ts +++ b/packages/components/link/link.component.spec.ts @@ -72,6 +72,21 @@ describe('KbqLink', () => { tick(); fixture.detectChanges(); + const link = fixture.nativeElement.querySelector('[kbq-link]'); + + expect(link.classList).not.toContain('kbq-link_print'); + // Unchanged from before the review: the href still lands in the attribute, only the class is absent. + expect(link.attributes.print.nodeValue).toContain('localhost:3003/'); + })); + + it('should treat an explicit undefined print as not printable', fakeAsync(() => { + const fixture = TestBed.createComponent(KbqLinkPrintTestApp); + + fixture.componentInstance.print.set(undefined); + fixture.detectChanges(); + tick(); + fixture.detectChanges(); + expect(fixture.nativeElement.querySelector('[kbq-link]').classList).not.toContain('kbq-link_print'); })); @@ -157,7 +172,7 @@ class KbqLinkBaseTestApp {} ` }) class KbqLinkPrintTestApp { - readonly print = signal(''); + readonly print = signal(''); } @Component({ diff --git a/packages/components/link/link.component.ts b/packages/components/link/link.component.ts index 0bbae2193c..76a0e6ae8b 100644 --- a/packages/components/link/link.component.ts +++ b/packages/components/link/link.component.ts @@ -3,7 +3,6 @@ import { AfterViewInit, booleanAttribute, computed, - contentChild, contentChildren, Directive, effect, @@ -100,10 +99,7 @@ export class KbqLink implements AfterViewInit, OnDestroy { readonly print = input(); /** @docs-private */ - protected readonly icon = contentChild(KbqIcon); - - /** @docs-private */ - protected readonly hasIcon = computed(() => !!this.icon()); + protected readonly hasIcon = computed(() => this.icons().length > 0); /** @docs-private */ protected readonly printMode = computed(() => this.print() != null); @@ -115,13 +111,20 @@ export class KbqLink implements AfterViewInit, OnDestroy { protected readonly printUrl = signal(undefined); constructor() { - effect(() => { + effect((onCleanup) => { const print = this.print(); - // `href` is DOM state rather than a signal, so it is read once the binding that sets it has landed. - Promise.resolve().then(() => - this.printUrl.set(print || this.nativeElement.href?.replace(baseURLRegex, '')) - ); + // `href` is DOM state rather than a signal, so it is read once the binding that sets it has + // landed — and dropped again if `print` changes or the view goes away before that. + let cancelled = false; + + onCleanup(() => (cancelled = true)); + + Promise.resolve().then(() => { + if (!cancelled) { + this.printUrl.set(print || this.nativeElement.href?.replace(baseURLRegex, '')); + } + }); }); // Icons projected asynchronously (e.g. behind an `@if`) update the `icons` signal after content diff --git a/packages/schematics/src/migrations/link-signals/README.md b/packages/schematics/src/migrations/link-signals/README.md index f65d2350aa..9e109be686 100644 --- a/packages/schematics/src/migrations/link-signals/README.md +++ b/packages/schematics/src/migrations/link-signals/README.md @@ -45,10 +45,11 @@ a different number for a disabled link. ## Notes with no call site to point at -- **An unbound link no longer carries `kbq-link_print`.** The old setter set - `printMode = value !== null` and only ran when the input was bound, so the class depended on - whether anyone bound `[print]` at all. It is driven by the input now and is absent until you bind - it. `print` accepts `string | null` instead of `any`. +- **`[print]="undefined"` no longer marks the link as printable.** The old setter tested + `value !== null`, so an explicit `undefined` passed it: the link got `kbq-link_print` and printed + its `href`. The input tests `!= null`, which covers both. An unbound link behaves exactly as + before — no class, and the href still lands in the `print` attribute. `print` accepts + `string | null` instead of `any`. - **Reading `disabled` reports the bound input.** The effective state — what the host bindings render — is `disabledSignal()`. The two only differ if something writes `disabledSignal` directly. diff --git a/packages/schematics/src/migrations/link-signals/data.ts b/packages/schematics/src/migrations/link-signals/data.ts index 99406bfab1..07db69bd57 100644 --- a/packages/schematics/src/migrations/link-signals/data.ts +++ b/packages/schematics/src/migrations/link-signals/data.ts @@ -25,13 +25,6 @@ export const WRITABLE_MEMBERS: ReadonlySet = new Set(); /** TypeScript type annotation that marks a receiver as a link. */ export const LINK_TYPE = 'KbqLink'; -/** - * Attribute selector whose template reference variables (`#ref="kbqLink"`) point at a link. The link is a - * directive rather than an element, so there is no element name to match — template reference rewriting - * only applies to ``-shaped hosts, which do not exist. Kept for the shared machinery. - */ -export const LINK_ELEMENT = 'kbq-link'; - /** Import specifier that marks a file as a link consumer. */ export const LINK_PACKAGE = '@koobiq/components/link'; @@ -44,12 +37,22 @@ export const LINK_PACKAGE = '@koobiq/components/link'; export const VALUE_CHANGED_MEMBERS: readonly string[] = ['tabIndex']; /** Members that moved out of the public surface and can no longer be read from outside the directive. */ -export const PROTECTED_MEMBERS: readonly string[] = ['icons', 'icon', 'hasIcon', 'printMode', 'printUrl']; +export const PROTECTED_MEMBERS: readonly string[] = [ + 'icons', + 'icon', + 'hasIcon', + 'printMode', + 'printUrl', + 'nativeElement', + 'destroyRef' +]; /** Appended to the protected-members warning. */ export const PROTECTED_HINT = 'They are the icon-spacing and print bookkeeping: the classes and the `print` attribute the directive ' + - 'puts on the anchor are the contract, not the state behind them.'; + 'puts on the anchor are the contract, not the state behind them. `icon` is gone \u2014 `icons` already ' + + 'answered the one question it was asked \u2014 `nativeElement` is private (call `getHostElement()`), and ' + + '`destroyRef` is gone: inject `DestroyRef` yourself if a subclass needs it.'; export interface WarnPattern { /** Owner of the member. The pattern is only evaluated for files that also name it. */ @@ -80,9 +83,10 @@ export const warnPatterns: WarnPattern[] = [ /** Printed once per project, after the per-file reports. */ export const SUMMARY = [ - ' `print` accepts `string | null` instead of `any`, and an unbound link no longer carries the ' + - '`kbq-link_print` class. The old setter set `printMode = value !== null`, so it only ever ran when ' + - 'the input was bound; the class is now driven by the input itself and is absent until you bind it.', + ' `print` accepts `string | null` instead of `any`, and `[print]="undefined"` no longer marks the link ' + + 'as printable. The old setter tested `value !== null`, so an explicit `undefined` passed it and added ' + + '`kbq-link_print` while printing the href; the input tests `!= null`, which covers both. An unbound ' + + 'link behaves exactly as before: no class, and the href still lands in the `print` attribute.', ' Reading `disabled` reports the bound input. The effective state — what the host bindings render — is ' + "`disabledSignal()`, which stays writable so `kbqTooltip`'s `forDisabledComponent` keeps working. " + 'The two only differ if something writes `disabledSignal` directly.' diff --git a/packages/schematics/src/migrations/link-signals/index.ts b/packages/schematics/src/migrations/link-signals/index.ts index 359407eeeb..aeff876a07 100644 --- a/packages/schematics/src/migrations/link-signals/index.ts +++ b/packages/schematics/src/migrations/link-signals/index.ts @@ -1,12 +1,9 @@ import { Path } from '@angular-devkit/core'; import { Rule, SchematicContext, Tree } from '@angular-devkit/schematics'; import ts from 'typescript'; -import { visitAll, Visitor } from '../../utils/ast'; import { logMessage } from '../../utils/messages'; import { setupOptions } from '../../utils/package-config'; -import { forEachClass, parseTemplate } from '../../utils/typescript'; import { - LINK_ELEMENT, LINK_PACKAGE, LINK_TYPE, PROTECTED_HINT, @@ -21,7 +18,6 @@ import { Schema } from './schema'; const LABEL = '[link-signals]'; const TS_EXT = '.ts'; -const HTML_EXT = '.html'; /** A text-span edit on the original file content. Applied right-to-left so offsets stay valid. */ interface Edit { @@ -38,10 +34,6 @@ interface Receiver { end: number; } -function escapeRegExp(value: string): string { - return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); -} - /** Applies text-span edits to `content`, right-to-left, so earlier edits don't shift later offsets. */ function applyEdits(content: string, edits: Edit[]): string { const sorted = [...edits].sort((a, b) => b.start - a.start || b.end - a.end); @@ -261,133 +253,6 @@ function warnReceiverMembers(context: SchematicContext, filePath: string, conten } } -/** Collects template reference variable names bound to a `` element. */ -class LinkRefCollector implements Visitor { - readonly refs = new Set(); - - visitElement(element: any): void { - if (element.name === LINK_ELEMENT) { - for (const attr of element.attrs ?? []) { - if (typeof attr.name !== 'string') continue; - - if (attr.name.startsWith('#')) this.refs.add(attr.name.slice(1)); - else if (attr.name.startsWith('ref-')) this.refs.add(attr.name.slice(4)); - } - } - - this.visitChildren(element); - } - - visitBlock(block: any): void { - this.visitChildren(block); - } - - private visitChildren(node: any): void { - for (const child of node.children ?? []) { - child.visit(this); - } - } - - visitAttribute(): void {} - visitText(): void {} - visitComment(): void {} - visitExpansion(): void {} - visitExpansionCase(): void {} - visitBlockParameter(): void {} - visitLetDeclaration(): void {} -} - -/** Rewrites `ref.member` reads to `ref.member()` for the given refs, scoped to those exact identifiers. */ -function rewriteRefReads(template: string, refs: string[]): { content: string; changed: boolean } { - const members = SIGNAL_MEMBERS.join('|'); - let content = template; - let changed = false; - - for (const ref of refs) { - // `\bref\.(member)\b(?!\s*\()` — skip anything already invoked, so the rewrite is idempotent. - const pattern = new RegExp(`\\b(${escapeRegExp(ref)})\\.(${members})\\b(?!\\s*\\()`, 'g'); - const next = content.replace(pattern, '$1.$2()'); - - if (next !== content) { - content = next; - changed = true; - } - } - - return { content, changed }; -} - -/** Pass B (core) — parse a template, discover link refs, rewrite their value-safe signal reads. */ -async function migrateTemplate(template: string): Promise<{ content: string; changed: boolean }> { - if (!template.includes(LINK_ELEMENT)) return { content: template, changed: false }; - - const parsed = await parseTemplate(template); - - if (!parsed.tree) return { content: template, changed: false }; - - const collector = new LinkRefCollector(); - - visitAll(collector, (parsed.tree as { rootNodes: unknown[] }).rootNodes); - - if (collector.refs.size === 0) return { content: template, changed: false }; - - return rewriteRefReads(template, [...collector.refs]); -} - -/** Interior `[start, end]` ranges of inline `@Component({ template: '…' })` string literals. */ -function collectInlineTemplateRanges(sourceFile: ts.SourceFile): Array<{ start: number; end: number }> { - const ranges: Array<{ start: number; end: number }> = []; - - forEachClass(sourceFile, (node) => { - const decorator = ts - .getDecorators(node) - ?.find( - (dec) => - ts.isCallExpression(dec.expression) && - ts.isIdentifier(dec.expression.expression) && - dec.expression.expression.text === 'Component' - ); - - if (!decorator || !ts.isCallExpression(decorator.expression)) return; - - const [arg] = decorator.expression.arguments; - - if (!arg || !ts.isObjectLiteralExpression(arg)) return; - - for (const prop of arg.properties) { - if ( - ts.isPropertyAssignment(prop) && - (ts.isIdentifier(prop.name) || ts.isStringLiteralLike(prop.name)) && - prop.name.text === 'template' && - ts.isStringLiteralLike(prop.initializer) && - prop.initializer.text - ) { - // +1 / -1 to exclude the opening/closing quote characters. - ranges.push({ start: prop.initializer.getStart(sourceFile) + 1, end: prop.initializer.getEnd() - 1 }); - } - } - }); - - return ranges; -} - -/** Pass B (inline) — rewrite link ref reads inside inline component templates. */ -async function migrateInlineTemplates(content: string, fileName: string): Promise { - const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS); - const ranges = collectInlineTemplateRanges(sourceFile).sort((a, b) => b.start - a.start); - let result = content; - - for (const { start, end } of ranges) { - const { content: rewritten, changed } = await migrateTemplate(result.slice(start, end)); - - if (changed) { - result = result.slice(0, start) + rewritten + result.slice(end); - } - } - - return result; -} - function logWarnings(context: SchematicContext, filePath: string, content: string): void { for (const { anchor, pattern, message } of warnPatterns) { if (!new RegExp(anchor).test(content) || !new RegExp(pattern).test(content)) continue; @@ -397,11 +262,11 @@ function logWarnings(context: SchematicContext, filePath: string, content: strin } /** - * A `.ts` file is a link consumer if it names any of the exported symbols, imports the package, or renders - * the element in an inline template — a component that only imports `KbqLinkModule` names no type. + * A `.ts` file is a link consumer if it names any of the exported symbols or imports the package. There is + * no element to look for: `kbq-link` is an attribute on an anchor. */ function referencesLink(content: string): boolean { - return /\bKbqLink\w*\b/.test(content) || content.includes(LINK_PACKAGE) || content.includes(`<${LINK_ELEMENT}`); + return /\bKbqLink\w*\b/.test(content) || content.includes(LINK_PACKAGE); } export default function linkSignals(options: Schema): Rule { @@ -412,13 +277,11 @@ export default function linkSignals(options: Schema): Rule { const rootDir = root ? tree.getDir(root as Path) : tree.root; const tsPaths: string[] = []; - const htmlPaths: string[] = []; rootDir.visit((filePath) => { if (filePath.includes('node_modules') || filePath.includes('/dist/')) return; if (filePath.endsWith(TS_EXT)) tsPaths.push(filePath); - else if (filePath.endsWith(HTML_EXT)) htmlPaths.push(filePath); }); let touched = 0; @@ -446,24 +309,7 @@ export default function linkSignals(options: Schema): Rule { logWarnings(context, filePath, original); warnReceiverMembers(context, filePath, original); - let content = migrateTsExpressions(original, filePath); - - content = await migrateInlineTemplates(content, filePath); - - commit(filePath, original, content); - } - - for (const filePath of htmlPaths) { - const original = tree.read(filePath)?.toString(); - - if (!original) continue; - - const { content, changed } = await migrateTemplate(original); - - if (changed) { - consumers++; - commit(filePath, original, content); - } + commit(filePath, original, migrateTsExpressions(original, filePath)); } // Nothing here uses the link, so the summary would only be noise. diff --git a/tools/public_api_guard/components/link.api.md b/tools/public_api_guard/components/link.api.md index 5bf38b7164..4e120adc0e 100644 --- a/tools/public_api_guard/components/link.api.md +++ b/tools/public_api_guard/components/link.api.md @@ -7,7 +7,6 @@ import { AfterViewInit } from '@angular/core'; import * as _angular_core from '@angular/core'; import * as i1 from '@angular/cdk/a11y'; -import { KbqIcon } from '@koobiq/components/icon'; import { OnDestroy } from '@angular/core'; import { Renderer2 } from '@angular/core'; @@ -25,7 +24,6 @@ export class KbqLink implements AfterViewInit, OnDestroy { getHostElement(): HTMLAnchorElement; protected readonly hasIcon: _angular_core.Signal; protected readonly hostTabIndex: _angular_core.Signal; - protected readonly icon: _angular_core.Signal; readonly multiline: _angular_core.InputSignalWithTransform; // (undocumented) ngAfterViewInit(): void; @@ -41,7 +39,7 @@ export class KbqLink implements AfterViewInit, OnDestroy { readonly tabIndex: _angular_core.InputSignalWithTransform; readonly useVisited: _angular_core.InputSignalWithTransform; // (undocumented) - static ɵdir: _angular_core.ɵɵDirectiveDeclaration; + static ɵdir: _angular_core.ɵɵDirectiveDeclaration; // (undocumented) static ɵfac: _angular_core.ɵɵFactoryDeclaration; }