fix(link)!: errors following a full review of the component (#DS-5482)

[artembelik]

What

A full review of link, in the same shape as the 20.3.0 component reviews. It also closes the two // @todo 20 markers the directive carried.

Three accessor inputs, each doing more than storing a value

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(); }

• tabIndex now reports what was bound. The host attribute still goes to -1 while the link is disabled — that moved into a hostTabIndex computed — so nothing about focus behavior changed; only a programmatic read sees the difference.
• print accepts string | null instead of any. 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.
• disabled is a plain booleanAttribute input, and the _disabled mirror kept in sync through a toObservable subscription is gone.

disabledSignal stays

kbqTooltip accepts a link through forDisabledComponent, typed Record<'disabledSignal', WritableSignal<boolean>>, and there is a docs example doing exactly that. So disabledSignal stays a public WritableSignal<boolean> — a linkedSignal over the input now, so binding [disabled] drives it and a direct write still wins. The host bindings read it rather than the input, which keeps that behavior identical.

Closed internals

icons, icon, hasIcon, printMode and printUrl left the public surface. icons is a signal query driven by an effect instead of a QueryList + changes subscription, so an icon projected behind an @if now gets its spacing class — the same fix the badge got.

Migration

link-signals runs from ng update @koobiq/components@20. It rewrites disabled reads to calls on receivers typed KbqLink and reports the rest. tabIndex is deliberately not rewritten: appending () would compile and hand back a different number for a disabled link.

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. The spec covers that decision.

Documented in docs/guides/migration.{en,ru}.md, section 18.

Testing

• link.component.spec.ts: 5 → 9 tests. New coverage for the disabled tab-order behavior, tabIndex reporting the bound value while disabled, a forDisabledComponent-style write to disabledSignal, and an unbound print.
• The existing print test needed its ordering corrected: it called tick() before the first detectChanges(), which only worked because the old constructor scheduled the microtask eagerly. In a real app the attribute has always landed on the change detection pass after the first, which is what the test asserts now.
• link-signals/index.spec.ts: 12 tests.
• Full packages/components (4997 tests) and packages/schematics (446 tests) suites pass.
• check-api is in sync.

BREAKING CHANGE

🤖 Generated with Claude Code

[github-actions]

Visit the preview URL for this PR (updated for commit 82b6959):

https://koobiq-next--prs-1983-svmq4h1h.web.app

_{(expires Sat, 05 Sep 2026 15:18:51 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: c9e37e518febda70d0317d07e8ceb35ac43c534c}

[lskramarov]

Review: link signal refactor + link-signals schematic

15 findings, most severe first. Full-effort pass over the component, the schematic, both migration guides and the API guard.

Two blockers:

1. index.ts:409 — the migration never writes anything. ng update invokes migrations with no options and migrations.json declares no schema, so fix is undefined. Every consumer gets "would update … (run with --fix)" and no rewrite — and --fix is not a flag ng update accepts. Twelve siblings already guard this with options.fix ?? true.
2. _link-theme.scss:194 — the kbq-link_print rule is dead CSS. The @media print block is nested in .kbq-link without a leading &, so it compiles to .kbq-link .kbq-link_print — a descendant selector for two classes on the same element. The whole print pipeline this PR reworked renders nothing. (Pre-existing; commented on link.component.ts:42 since the SCSS is outside the diff.)

Schematic correctness: the template pass can never fire (kbq-link matched as an element name, not an attribute), compound assignments are rewritten into invalid syntax, shadowed receivers are rewritten, and this.link!.disabled — the spelling strictPropertyInitialization forces — is silently skipped by both the rewrite and the warnings.

Component: printMode loosened !== to !=, the disabled anchor has no aria-disabled, icon edge detection regresses under preserveWhitespaces, and printUrl goes stale on [href] changes.

Docs: the bolded "unbound link no longer carries kbq-link_print" change did not happen (old code behaved identically), a table cell is truncated mid-sentence, and the rewritten wave intro contradicts its own subsection list.

🤖 Generated with Claude Code

[lskramarov]

fix is undefined under ng update, so the migration never writes anything.

@angular/cli runs migrations as this.executeSchematic(workflow, migration.collection.name, migration.name) — no options argument, so options = {}. And @angular-devkit/schematics/tools/schema-option-transform.js applies schema defaults only if (schematic.schema && schematic.schemaJson); the link-signals entry in migrations.json correctly has no schema key, so schema.json's "default": true never reaches the rule.

Result: ng update @koobiq/components@20 prints [link-signals] would update src/app/x.ts (run with --fix to apply) for every consumer file and rewrites nothing — while migration.en.md:1042 promises "Every schematic named below runs automatically" and :1070 says "the disabled reads are rewritten". The suggested --fix flag does not exist on ng update.

Twelve siblings already guard this, app-switcher-signals/index.ts:413 with a comment spelling out the trap:

Suggested change

	const { project, fix } = options;
	const { project } = options;
	const fix = options.fix ?? true;

[lskramarov]

The kbq-link_print class this binding drives is dead CSS — nothing consumes it.

_link-theme.scss:193-194 nests the @media print block inside .kbq-link { … } but writes .kbq-link_print:not(.kbq-disabled) without a leading &, so it compiles to a descendant selector. Verified with the repo's own sass:

@media print {
  .kbq-link .kbq-link_print:not(.kbq-disabled)::after { content: ":\a" attr(print); }
}

Both classes sit on the same host element, so the rule can never match. Every other same-element class in that file uses &. (&.kbq-link_big, &.kbq-disabled, &.kbq-link_pseudo, &.cdk-keyboard-focused) — this is the sole exception, and it is the only @media print rule in packages/ that reads the class or the print attribute.

So for the shipped link-print-example, printing the page appends no URL: the directive sets class="kbq-link_print" and print="…" correctly, but no rule consumes them. That makes printMode, the printUrl signal, the new should drop .kbq-link_print when print is unbound spec, and the whole kbq-link_print migration note unobservable to any user. Pre-existing, but this PR reworked the entire print pipeline on top of it. Fix in _link-theme.scss:194: &.kbq-link_print:not(.kbq-disabled).

[lskramarov]

LINK_ELEMENT is matched as an element name, so the whole template pass is unreachable.

KbqLink is the attribute selector [kbq-link] on an <a>, so element.name is always a and collector.refs is always empty. data.ts:31 acknowledges this ("Kept for the shared machinery") — but the machinery is a private copy in this file, not shared, so there is nothing to keep it for.

Two concrete costs:

1. Template reads are silently missed. <a #l="kbqLink" kbq-link [disabled]="d"></a> @if (l.disabled) { … } — a pattern this repo uses twice (packages/components/tooltip/tooltip.spec.ts:927, packages/docs-examples/components/tooltip/tooltip-disabled-for-component/tooltip-disabled-for-component-example.ts:28). After the migration l.disabled is an InputSignalWithTransform function object: permanently truthy in a boolean position, no template type-check error, no rewrite — and no warning either, since logWarnings/warnReceiverMembers run only over .ts files.
2. Wasted work. migrateTemplate's guard at :322 is template.includes('kbq-link'), which is true for every real <a kbq-link> consumer, so parseTemplate runs a full @angular/compiler parse on every such .html and every inline template and throws the result away (23 .html files, ~171 KB, in this repo alone).

Either match the attribute the way button-truncation/index.ts:86 does (hasAttr(node, BUTTON_ATTR)) and rewrite the ref reads, or delete the pass along with LINK_ELEMENT, LinkRefCollector, rewriteRefReads, migrateTemplate, migrateInlineTemplates, escapeRegExp, the htmlPaths loop, and the content.includes('<kbq-link') disjunct at :404.

[lskramarov]

Compound assignments and increments fall through to the read branch and produce a file that no longer parses.

The write guard at :156 returns early only for ts.SyntaxKind.EqualsToken. link.disabled ||= this.formDisabled is a BinaryExpression with BarBarEqualsToken, so it reaches this line and the schematic writes:

link.disabled() ||= this.formDisabled;   // TS2364: left-hand side must be a variable or property access

Identical for ??=, &&=, +=, and link.disabled++ → link.disabled()++ (a PostfixUnaryExpression, matched by none of the three guards). Once fix actually defaults to true, ng update writes this to disk — replacing a clear "cannot assign to a read-only input" error with a syntax-shaped one.

Widen the guard to ts.isAssignmentExpression(parent) (or an operator-token range check) and add prefix/postfix unary handling.

[lskramarov]

Receiver scopes are flat ranges, so a shadowing inner binding of the same name is rewritten.

A parameter's scope is the whole enclosing function; this.<name> (:117, :123) covers the whole class; a typed top-level local covers the whole SourceFile. Nothing re-resolves the identifier at the access site.

read(link: KbqLink) {
    this.buttons.forEach((link: KbqButton) => console.log(link.disabled)); // → link.disabled()
    return link.disabled;
}

The inner link is a KbqButton, whose disabled is a plain boolean getter, but it lies inside the outer parameter's range with the same source text, so both reads get (). Result: a compile error, or a runtime TypeError: link.disabled is not a function when the callback param is untyped. Same for a for (const link of this.buttons) loop inside such a method, and for a class declared inside a method of a class that has link: KbqLink.

[lskramarov]

The effect branches on DOM state it does not track.

The only reactive dependency is this.icons(), but the gate at :160 (filteredNodesWithoutComments.length > 1) and the findIndex positioning both read nativeElement.childNodes imperatively.

<a kbq-link><i kbq-icon="kbq-plus_16"></i>@if (label()) { {{ label() }} }</a>: with label() false the non-comment children are [i] — length 1, gate false, no class. Flipping label() to true makes them [i, text], which should give the icon kbq-icon_left — but icons() did not change, so the effect never re-runs and the icon stays unspaced. Symmetrically, a stale kbq-icon_right survives when the sibling disappears.

The comment added at :127-128 says the assignment now reacts to the signal; it reacts to the icon query only, not to the sibling nodes the branch actually reads. Inherited from the old icons.changes subscription, but the function was rewritten here and neither spacing class has a test — the spec only covers kbq-text-with-icon, which comes from the hasIcon computed.

[lskramarov]

printUrl derives from href but only tracks print(), and moving it out of the constructor added a second failure mode.

1. Stale on [href] change. <a kbq-link [href]="url()" [print]> with url() going …/1 → …/2: print() never changes, the effect never re-runs, and the printed page shows the old URL next to the new link text — silently, while the documented default is "the href without its protocol".
2. Never computed in a detached view. This effect() resolves a ViewContext, so it is a view effect that runs only from runEffectsInView inside refreshView. Under ChangeDetectorRef.detach(), or in a view not attached to ApplicationRef, printUrl is now never set — the old constructor-scheduled microtask always ran.

The repo already uses the sanctioned primitive for a deferred DOM read: afterNextRender at ~65 call sites in packages/components, e.g. core/form-field/field-sizing-content.ts:43 (same kbqInjectNativeElement + deferred read). It is tied to the injection context, runs in the render phase rather than an untracked microtask, and does not run during SSR — and it would drop the extra fixture.detectChanges() the spec had to add at :51 and :63.

[lskramarov]

PROTECTED_MEMBERS misses several members that left the public surface, and the message's wording is wrong for icons.

Diffing git show 4bb1c9927:tools/public_api_guard/components/link.api.md against the new file, these also left the surface with no entry here, no warnPatterns rule, and no row in either migration table:

• protected readonly nativeElement: HTMLElement → now private
• protected readonly destroyRef: DestroyRef → deleted
• ngAfterContentInit(): void → deleted, along with implements AfterContentInit
• set tabIndex(value: number) → gone; link.tabIndex = 3 no longer compiles, while the tabIndex warning text only discusses reading

class MyLink extends KbqLink { ngAfterContentInit() { super.ngAfterContentInit(); this.renderer.setAttribute(this.nativeElement, …); } } breaks on three members with zero guidance.

Separately: link.component.ts:58 declares icons private, not protected, so the message emitted at index.ts:258 ("These KbqLink members are now protected") is wrong for it. Both doc tables get this right ("Now protected/private"); only the runtime message a consumer actually sees does not.

[lskramarov]

This breaking change never happened.

In the old component printMode: boolean; had no initializer and was written only by set print. For <a kbq-link> with no binding the setter never ran, so printMode === undefined → [class.kbq-link_print] falsy → class absent. New: print() is undefined → undefined != null is false → class absent. Identical.

Consequences:

• Consumers are told to audit every unbound link for a change that did not occur.
• The new spec should drop .kbq-link_print when print is unbound (link.component.spec.ts:68) passes verbatim against the pre-PR component, so it pins nothing.
• The shape that did change — [print] bound to an undefined-valued expression, which used to turn print mode on — is documented nowhere.

The same claim is repeated in docs/guides/migration.ru.md:1072, packages/schematics/src/migrations/link-signals/README.md:48, data.ts:82 (the SUMMARY printed to every consumer), and the descriptions in both collection.json and migrations.json.

[lskramarov]

Truncated cell — the sentence ends on a dangling verb: "the kbq-link_print class and print attribute are".

The RU counterpart at migration.ru.md:1068 is complete ("контракт — класс kbq-link_print и атрибут print"), so the predicate was lost only on the EN side. packages/schematics/src/migrations/link-signals/README.md:40 carries the identical truncated cell.

The suggestion spans the whole table because prettier realigns every row once the last cell grows — this is prettier --parser markdown --print-width 120 output, so it is check-clean:

Suggested change

	| 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 |
	| 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 contract |

---

Separately, one line up in the same section — migration.en.md:1038 contradicts its own subsection list. It now reads:

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.

But the subsections under ### 18 are Link (1052), Search expandable (1072), Split button (1094) and Title (1116) — three of the four are named in the wave-1 list in the same sentence. Only Link belongs to the second wave. migration.ru.md:1042 carries the identical error, so both files are in sync and both wrong; the sentence this replaced was consistent.