Sitelet https://github.com/angular/angular/commit/db91010128bd82f865f885a82a4eae25ec5c8e78
Skip to content

Commit db91010

Browse files
erkamyamanNothingEverHappens
erkamyaman
authored and
NothingEverHappens
committed
fix(docs-infra): stop generating member anchors that do not exist
When `{@link Foo}` does not resolve, the renderer falls back to `CurrentSymbol.Foo` without checking that `Foo` is a member of the symbol being rendered, so it emits a URL to an anchor that does not exist. `DefaultExport` is rendered in both core and router while `injectAsync` is only in core's entries, so /api/router/DefaultExport links `injectAsync` to /api/router/DefaultExport#injectAsync. Resolve the symbol first, and only fall back to the member form when the symbol really is a member. A property on a function entry, and an initializer API function, no longer take a fragment at all. Make the `@see` on `DefaultExport` an absolute link so it resolves from every package that renders it. Two consequences worth calling out. A `{@link}` that resolves to nothing now fails the docs build where it used to emit a broken anchor, including a core-only symbol referenced from a type another package re-exports. And an explicit `{@link}` no longer consults the auto-linking exemption list, which is what lets `{@link Event}` on `RouterEvent` reach /api/router/Event instead of a fragment on itself. (cherry picked from commit c863e00)
1 parent 74bc86d commit db91010

10 files changed

Lines changed: 135 additions & 20 deletions

File tree

‎adev/shared-docs/pipeline/api-gen/rendering/index.mts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,12 @@ import {DocEntry} from './entities.mjs';
1313
import {isCliEntry, isHiddenEntry} from './entities/categorization.mjs';
1414
import {getRenderable} from './processing.mjs';
1515
import {renderEntry} from './rendering.mjs';
16-
import {setCurrentSymbol, setSymbolMembers, setSymbols} from './symbol-context.mjs';
16+
import {
17+
setCurrentSymbol,
18+
setSymbolEntryTypes,
19+
setSymbolMembers,
20+
setSymbols,
21+
} from './symbol-context.mjs';
1722
import {setDefinedRoutes} from './defined-routes-context.mjs';
1823
import {CliCommandRenderable, DocEntryRenderable} from './entities/renderables.mjs';
1924
import {initHighlighter} from '../../shared/shiki.mjs';
@@ -153,6 +158,13 @@ async function main() {
153158
// Setting the symbols are a global context for the rendering templates of this entry
154159
setSymbols(collection.symbols);
155160
setSymbolMembers(buildSymbolMembersIndex(collection.entries));
161+
setSymbolEntryTypes(
162+
new Map(
163+
collection.entries
164+
.filter((entry): entry is DocEntry => 'entryType' in entry)
165+
.map((entry) => [entry.name, entry.entryType]),
166+
),
167+
);
156168

157169
const renderableEntries: (DocEntryRenderable | CliCommandRenderable)[] = [];
158170
for (const entry of extractedEntries) {

‎adev/shared-docs/pipeline/api-gen/rendering/symbol-context.mts‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {getSymbolUrl as sharedGetSymbolUrl} from '../../shared/linking.mjs';
9+
import {
10+
getSymbolUrl as sharedGetSymbolUrl,
11+
resolveSymbolUrl as sharedResolveSymbolUrl,
12+
} from '../../shared/linking.mjs';
1013

1114
/**
1215
* API pages are generated each package at a time.
@@ -25,18 +28,20 @@ let symbols: Record<string, string> = {};
2528
*/
2629
let symbolMembers: Map<string, Set<string>> = new Map();
2730

31+
let symbolEntryTypes: Map<string, string> = new Map();
32+
2833
// This is used to store the currently processed symbol (usually a class or an interface)
2934
let currentSymbol: string | undefined;
3035
export function setCurrentSymbol(symbol: string): void {
3136
currentSymbol = symbol;
3237
}
3338

3439
/** Convert Record<string, string> to ApiEntries format */
35-
export function getSymbolsAsApiEntries(): Record<string, {moduleName: string}> {
36-
const result: Record<string, {moduleName: string}> = {};
40+
export function getSymbolsAsApiEntries(): Record<string, {moduleName: string; entryType?: string}> {
41+
const result: Record<string, {moduleName: string; entryType?: string}> = {};
3742

3843
for (const symbol in symbols) {
39-
result[symbol] = {moduleName: symbols[symbol]};
44+
result[symbol] = {moduleName: symbols[symbol], entryType: symbolEntryTypes.get(symbol)};
4045
}
4146

4247
return result;
@@ -50,6 +55,10 @@ export function setSymbols(newSymbols: Record<string, string>): void {
5055
symbols = newSymbols;
5156
}
5257

58+
export function setSymbolEntryTypes(newSymbolEntryTypes: Map<string, string>): void {
59+
symbolEntryTypes = newSymbolEntryTypes;
60+
}
61+
5362
/** Set the index of known members per symbol for the currently processed package. */
5463
export function setSymbolMembers(newSymbolMembers: Map<string, Set<string>>): void {
5564
symbolMembers = newSymbolMembers;
@@ -64,6 +73,10 @@ export function getSymbolurl(symbol: string): string | undefined {
6473
return sharedGetSymbolUrl(symbol, getSymbolsAsApiEntries());
6574
}
6675

76+
export function resolveSymbolUrl(symbol: string): string | undefined {
77+
return sharedResolveSymbolUrl(symbol, getSymbolsAsApiEntries());
78+
}
79+
6780
export function unknownSymbolMessage(link: string, symbol: string): string {
6881
return `WARNING: {@link ${link}} is invalid, ${symbol} or ${currentSymbol}.${symbol} is unknown in this context`;
6982
}

‎adev/shared-docs/pipeline/api-gen/rendering/test/transforms/jsdoc-transforms.spec.mts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,35 @@ describe('jsdoc transforms', () => {
267267
expect(entryFn).toThrowError(/Broken link.*RouterModule has no member named 'forBogus'/);
268268
});
269269

270+
it('should not treat an unknown name as a member of the current symbol', () => {
271+
setCurrentSymbol('RouterEvent');
272+
setSymbols({RouterEvent: 'router'});
273+
setSymbolMembers(new Map([['RouterEvent', new Set(['id', 'url'])]]));
274+
275+
const entryFn = () =>
276+
addHtmlAdditionalLinks({
277+
jsdocTags: [{name: 'see', comment: '{@link Event}'}],
278+
moduleName: 'test',
279+
});
280+
281+
expect(entryFn).toThrow(
282+
'WARNING: {@link Event} is invalid, Event or RouterEvent.Event is unknown in this context',
283+
);
284+
});
285+
286+
it('should link symbols that are only exempt from auto-linking', () => {
287+
setCurrentSymbol('AnimationPlayer');
288+
setSymbols({AnimationPlayer: 'animations', animate: 'animations'});
289+
setSymbolMembers(new Map([['AnimationPlayer', new Set(['play'])]]));
290+
291+
const entry = addHtmlAdditionalLinks({
292+
jsdocTags: [{name: 'see', comment: '{@link animate}'}],
293+
moduleName: 'test',
294+
});
295+
296+
expect(entry.additionalLinks[0].url).toBe('/api/animations/animate');
297+
});
298+
270299
it('should throw on a miscased #member fragment for a known API symbol', () => {
271300
setSymbols({RouterModule: 'router'});
272301
setSymbolMembers(new Map([['RouterModule', new Set(['forRoot', 'forChild'])]]));

‎adev/shared-docs/pipeline/api-gen/rendering/transforms/jsdoc-transforms.mts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import {
3232
getSymbolMembers,
3333
getSymbolsAsApiEntries,
3434
getSymbolUrl,
35+
resolveSymbolUrl,
3536
unknownSymbolMessage,
3637
} from '../symbol-context.mjs';
3738
import {getAnchorsForRoute, hasDefinedRoutes, isKnownRoute} from '../defined-routes-context.mjs';
@@ -353,13 +354,18 @@ function parseAtLink(link: string): {label: string; url: string} | undefined {
353354
};
354355
}
355356

356-
let url = getSymbolUrl(rawSymbol);
357+
let url = resolveSymbolUrl(rawSymbol);
357358
const label = description ?? rawSymbol;
358359

359360
if (!url) {
360361
const currentSymbol = getCurrentSymbol();
361362
// 2nd attempt, try to get the module name in the context of the current symbol
362-
url = getSymbolUrl(`${currentSymbol}.${rawSymbol}`);
363+
const members = currentSymbol ? getSymbolMembers(currentSymbol) : undefined;
364+
// Normalize the same way `extractFromSymbol` does, so a member written with a call, generics
365+
// or a non-null assertion is still recognized as a member.
366+
if (!members || members.has(rawSymbol.replace(/[(<!].*$/, ''))) {
367+
url = resolveSymbolUrl(`${currentSymbol}.${rawSymbol}`);
368+
}
363369

364370
if (!url || !currentSymbol) {
365371
throw unknownSymbolMessage(link, rawSymbol);

‎adev/shared-docs/pipeline/guides/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ ts_project(
7979
deps = [
8080
":guides_lib",
8181
"//adev:node_modules/@types/node",
82+
"//adev/shared-docs/pipeline/shared:linking",
8283
"//adev/shared-docs/pipeline/shared:shiki",
8384
"//adev/shared-docs/pipeline/shared/marked",
8485
],

‎adev/shared-docs/pipeline/guides/index.mts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,14 @@
99
import {readFile, writeFile} from 'fs/promises';
1010
import path from 'path';
1111
import {parseMarkdownAsync} from '../shared/marked/parse.mjs';
12+
import {ApiEntries} from '../shared/linking.mjs';
1213
import {initHighlighter} from '../shared/shiki.mjs';
1314
import {hasUnknownAnchors} from './helpers.mjs';
1415

1516
type ApiManifest = ApiManifestPackage[];
1617
interface ApiManifestPackage {
1718
moduleName: string;
18-
entries: {name: string; aliases?: string[]}[];
19+
entries: {name: string; type?: string; aliases?: string[]}[];
1920
}
2021

2122
async function main() {
@@ -90,12 +91,10 @@ async function main() {
9091

9192
main();
9293

93-
function mapManifestToEntries(
94-
apiManifest: ApiManifest,
95-
): Record<string, {moduleName: string; targetSymbol?: string}> {
94+
function mapManifestToEntries(apiManifest: ApiManifest): ApiEntries {
9695
const duplicateEntries = new Set<string>();
9796

98-
const entryToModuleMap: Record<string, {moduleName: string; targetSymbol?: string}> = {};
97+
const entryToModuleMap: ApiEntries = {};
9998
for (const pkg of apiManifest) {
10099
for (const entry of pkg.entries) {
101100
if (duplicateEntries.has(entry.name)) {
@@ -106,14 +105,15 @@ function mapManifestToEntries(
106105
} else {
107106
const normalizedModuleName = pkg.moduleName.replace(/^@angular\//, '');
108107

109-
entryToModuleMap[entry.name] = {moduleName: normalizedModuleName};
108+
entryToModuleMap[entry.name] = {moduleName: normalizedModuleName, entryType: entry.type};
110109

111110
// If there are aliases, create entries for each alias
112111
if (entry.aliases) {
113112
for (const alias of entry.aliases) {
114113
entryToModuleMap[alias] = {
115114
moduleName: normalizedModuleName,
116115
targetSymbol: entry.name,
116+
entryType: entry.type,
117117
};
118118
}
119119
}

‎adev/shared-docs/pipeline/shared/linking.mts‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,10 @@ export function shouldLinkSymbol(symbol: string): boolean {
4646
}
4747

4848
// symbolName -> symbol info with moduleName and optional targetSymbol for aliases
49-
export type ApiEntries = Record<string, {moduleName: string; targetSymbol?: string}>;
49+
export type ApiEntries = Record<
50+
string,
51+
{moduleName: string; targetSymbol?: string; entryType?: string}
52+
>;
5053

5154
/**
5255
* Extracts the symbol name and property name from a symbol string.
@@ -73,11 +76,19 @@ export function extractFromSymbol(symbol: string): {propName: string | null; sym
7376
[symbolName, propName] = symbolName.split('.');
7477
}
7578

76-
return {propName: propName ?? null, symbolName: symbolName};
79+
return {propName: propName?.replace(/[(<!].*$/, '') ?? null, symbolName: symbolName};
7780
}
7881

7982
export function getSymbolUrl(symbol: string, apiEntries: ApiEntries): string | undefined {
80-
if (hasMoreThanOneDot(symbol) || !shouldLinkSymbol(symbol)) {
83+
if (!shouldLinkSymbol(symbol)) {
84+
return undefined;
85+
}
86+
87+
return resolveSymbolUrl(symbol, apiEntries);
88+
}
89+
90+
export function resolveSymbolUrl(symbol: string, apiEntries: ApiEntries): string | undefined {
91+
if (hasMoreThanOneDot(symbol)) {
8192
return undefined;
8293
}
8394

@@ -88,10 +99,16 @@ export function getSymbolurl(symbol: string, apiEntries: ApiEntries): string | u
8899
}
89100

90101
const apiEntry = apiEntries[symbolName];
102+
if (propName && apiEntry.entryType === 'function') {
103+
return undefined;
104+
}
105+
91106
const moduleName = apiEntry.moduleName;
92107
const targetSymbol = apiEntry.targetSymbol ?? symbolName;
108+
const fragment =
109+
propName && apiEntry.entryType !== 'initializer_api_function' ? `#${propName}` : '';
93110

94-
return `/api/${moduleName}/${targetSymbol}${propName ? `#${propName}` : ''}`;
111+
return `/api/${moduleName}/${targetSymbol}${fragment}`;
95112
}
96113

97114
function hasMoreThanOneDot(str: string) {

‎adev/shared-docs/pipeline/shared/test/linking.spec.mts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,41 @@ describe('getSymbolUrl', () => {
6262
expect(getSymbolUrl('unknownSelector', apiEntries)).toBeUndefined();
6363
expect(getSymbolUrl('ngUnknownDirective', apiEntries)).toBeUndefined();
6464
});
65+
66+
it('should only keep the member name in the fragment', () => {
67+
const apiEntries = {
68+
By: {moduleName: 'platform-browser'},
69+
TestBed: {moduleName: 'core/testing'},
70+
DebugElement: {moduleName: 'core'},
71+
};
72+
73+
expect(getSymbolUrl(`By.css('h2:not([highlight])')`, apiEntries)).toBe(
74+
'/api/platform-browser/By#css',
75+
);
76+
expect(getSymbolUrl('TestBed.createComponent<T>', apiEntries)).toBe(
77+
'/api/core/testing/TestBed#createComponent',
78+
);
79+
expect(getSymbolUrl('DebugElement.query(predicate)!', apiEntries)).toBe(
80+
'/api/core/DebugElement#query',
81+
);
82+
});
83+
84+
it('should not link a property of a plain function', () => {
85+
const apiEntries = {
86+
state: {moduleName: 'animations', entryType: 'function'},
87+
schema: {moduleName: 'forms/signals', entryType: 'function'},
88+
};
89+
90+
expect(getSymbolUrl('state.metadata(HELP)', apiEntries)).toBeUndefined();
91+
expect(getSymbolUrl('schema.json', apiEntries)).toBeUndefined();
92+
expect(getSymbolUrl('schema', apiEntries)).toBe('/api/forms/signals/schema');
93+
});
94+
95+
it('should link a property of an initializer API function to its page', () => {
96+
const apiEntries = {
97+
model: {moduleName: 'core', entryType: 'initializer_api_function'},
98+
};
99+
100+
expect(getSymbolUrl('model.required', apiEntries)).toBe('/api/core/model');
101+
});
65102
});

‎packages/core/src/util/default_export.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
/**
1010
* An ES Module object with a default export of the given type.
1111
*
12-
* @see {@link injectAsync}
12+
* @see [injectAsync](/api/core/injectAsync)
1313
* @see [Route#loadComponent](/api/router/Route#loadComponent)
1414
* @see [LoadChildrenCallback](/api/router/LoadChildrenCallback)
1515
*

‎packages/forms/signals/compat/src/api/extract.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export interface ExtractFilter {
5454
}
5555

5656
/**
57-
* Utility to unwrap a {@link FieldTree} into its underlying raw value.
57+
* Utility to unwrap a {@link /api/forms/signals/FieldTree FieldTree} into its underlying raw value.
5858
*
5959
* This function is recursive, so if the field tree represents an object or an array,
6060
* the result will be an object or an array of the raw values of its children.
@@ -67,7 +67,7 @@ export interface ExtractFilter {
6767
*/
6868
export function extractValue<T>(field: FieldTree<T>): RawValue<T>;
6969
/**
70-
* Utility to unwrap a {@link FieldTree} into its underlying raw value.
70+
* Utility to unwrap a {@link /api/forms/signals/FieldTree FieldTree} into its underlying raw value.
7171
*
7272
* This function is recursive, so if the field tree represents an object or an array,
7373
* the result will be an object or an array of the raw values of its children.

0 commit comments

Comments
 (0)