Skip to content

Commit

Permalink
Improve handling of external link resolvers
Browse files Browse the repository at this point in the history
Noticed when investing adding symbol ID handling in
typedoc-plugin-mdn-links as originally noted was missing in #2700.
  • Loading branch information
Gerrit0 committed Sep 19, 2024
1 parent e0e5923 commit 9189a4c
Show file tree
Hide file tree
Showing 9 changed files with 120 additions and 24 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Unreleased

### Bug Fixes

- Correctly handle external link resolver link text when referencing an external symbol, #2700.
- Corrected handling of `@link` tags present in comments at the start of source files.

## v0.26.7 (2024-09-09)

### Features
Expand Down
12 changes: 12 additions & 0 deletions src/lib/converter/comments/discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,18 @@ function findJsDocForComment(
.getJSDocCommentsAndTags(node)
.map((doc) => ts.findAncestor(doc, ts.isJSDoc)) as ts.JSDoc[];

if (ts.isSourceFile(node)) {
if (node.statements.length) {
jsDocs.push(
...(ts
.getJSDocCommentsAndTags(node.statements[0])
.map((doc) =>
ts.findAncestor(doc, ts.isJSDoc),
) as ts.JSDoc[]),
);
}
}

return jsDocs.find((doc) => doc.pos === ranges[0].pos);
}
}
Expand Down
19 changes: 11 additions & 8 deletions src/lib/converter/comments/linkResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,23 +141,26 @@ function resolveLinkTag(
defaultDisplayText =
part.tsLinkText ||
(options.preserveLinkText ? part.text : target.name);
} else if (declRef) {
} else {
// If we didn't find a target, we might be pointing to a symbol in another project that will be merged in
// or some external symbol, so ask external resolvers to try resolution. Don't use regular declaration ref
// resolution in case it matches something that would have been merged in later.
if (declRef) {
pos = declRef[1];
}

const externalResolveResult = externalResolver(
declRef[0],
declRef?.[0] ?? part.target.toDeclarationReference(),
reflection,
part,
part.target instanceof ReflectionSymbolId
? part.target
: undefined,
part.target,
);

defaultDisplayText = options.preserveLinkText
? part.text
: part.text.substring(0, pos);
defaultDisplayText =
part.tsLinkText ||
(options.preserveLinkText
? part.text
: part.text.substring(0, pos));

switch (typeof externalResolveResult) {
case "string":
Expand Down
4 changes: 4 additions & 0 deletions src/lib/converter/comments/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,10 @@ function inlineTag(
};
if (tagName.tsLinkTarget) {
inlineTag.target = tagName.tsLinkTarget;
}
// Separated from tsLinkTarget to avoid storing a useless empty string
// if TS doesn't have an opinion on what the link text should be.
if (tagName.tsLinkText) {
inlineTag.tsLinkText = tagName.tsLinkText;
}
block.push(inlineTag);
Expand Down
23 changes: 22 additions & 1 deletion src/lib/models/reflections/ReflectionSymbolId.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,16 @@ import { existsSync } from "fs";
import { isAbsolute, join, relative, resolve } from "path";
import ts from "typescript";
import type { JSONOutput, Serializer } from "../../serialization/index";
import { getCommonDirectory, readFile } from "../../utils/fs";
import {
findPackageForPath,
getCommonDirectory,
readFile,
} from "../../utils/fs";
import { normalizePath } from "../../utils/paths";
import { getQualifiedName } from "../../utils/tsutils";
import { optional, validate } from "../../utils/validation";
import type { DeclarationReference } from "../../converter/comments/declarationReference";
import { splitUnquotedString } from "./utils";

/**
* See {@link ReflectionSymbolId}
Expand Down Expand Up @@ -78,6 +84,21 @@ export class ReflectionSymbolId {
}
}

toDeclarationReference(): DeclarationReference {
return {
resolutionStart: "global",
moduleSource: findPackageForPath(this.fileName),
symbolReference: {
path: splitUnquotedString(this.qualifiedName, ".").map(
(path) => ({
navigation: ".",
path,
}),
),
},
};
}

toObject(serializer: Serializer) {
const sourceFileName = isAbsolute(this.fileName)
? normalizePath(
Expand Down
15 changes: 0 additions & 15 deletions src/lib/models/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -928,21 +928,6 @@ export class ReferenceType extends Type {
.fileName.replace(/\\/g, "/");
if (!symbolPath) return ref;

// Attempt to decide package name from path if it contains "node_modules"
let startIndex = symbolPath.lastIndexOf("node_modules/");
if (startIndex !== -1) {
startIndex += "node_modules/".length;
let stopIndex = symbolPath.indexOf("/", startIndex);
// Scoped package, e.g. `@types/node`
if (symbolPath[startIndex] === "@") {
stopIndex = symbolPath.indexOf("/", stopIndex + 1);
}
const packageName = symbolPath.substring(startIndex, stopIndex);
ref.package = packageName;
return ref;
}

// Otherwise, look for a "package.json" file in a parent path
ref.package = findPackageForPath(symbolPath);
return ref;
}
Expand Down
12 changes: 12 additions & 0 deletions src/lib/utils/fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,18 @@ export function discoverPackageJson(dir: string) {
const packageCache = new Map<string, string>();

export function findPackageForPath(sourcePath: string): string | undefined {
// Attempt to decide package name from path if it contains "node_modules"
let startIndex = sourcePath.lastIndexOf("node_modules/");
if (startIndex !== -1) {
startIndex += "node_modules/".length;
let stopIndex = sourcePath.indexOf("/", startIndex);
// Scoped package, e.g. `@types/node`
if (sourcePath[startIndex] === "@") {
stopIndex = sourcePath.indexOf("/", stopIndex + 1);
}
return sourcePath.substring(startIndex, stopIndex);
}

const dir = dirname(sourcePath);
const cache = packageCache.get(dir);
if (cache) {
Expand Down
6 changes: 6 additions & 0 deletions src/test/converter2/issues/gh2700.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* {@link Map.size | size user specified}
* {@link Map.size user specified}
* {@link Map.size}
*/
export const abc = new Map<string, number>();
48 changes: 48 additions & 0 deletions src/test/issues.c2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
QueryType,
ReferenceReflection,
ReflectionKind,
ReflectionSymbolId,
ReflectionType,
SignatureReflection,
UnionType,
Expand Down Expand Up @@ -1732,4 +1733,51 @@ describe("Issue Tests", () => {
[undefined, "2", '"counterclockwise"'],
);
});

it("#2700a correctly parses links to global properties", () => {
const project = convert();
app.options.setValue("validation", {
invalidLink: true,
notDocumented: false,
notExported: false,
});

app.validate(project);
logger.expectMessage(
'warn: Failed to resolve link to "Map.size | size user specified" in comment for abc',
);
logger.expectMessage(
'warn: Failed to resolve link to "Map.size user specified" in comment for abc',
);
logger.expectMessage(
'warn: Failed to resolve link to "Map.size" in comment for abc',
);

const abc = query(project, "abc");
const link = abc.comment?.summary.find((c) => c.kind === "inline-tag");
ok(link?.target instanceof ReflectionSymbolId);
});

it("#2700b respects user specified link text when resolving external links", () => {
const project = convert();

const abc = query(project, "abc");
ok(abc.comment);

const resolvers = app.converter["_externalSymbolResolvers"].slice();
app.converter.addUnknownSymbolResolver(() => {
return {
target: "https://typedoc.org",
caption: "resolver caption",
};
});
app.converter.resolveLinks(abc.comment, abc);
app.converter["_externalSymbolResolvers"] = resolvers;

equal(getLinks(abc), [
{ display: "size user specified", target: "https://typedoc.org" },
{ display: "user specified", target: "https://typedoc.org" },
{ display: "resolver caption", target: "https://typedoc.org" },
]);
});
});

0 comments on commit 9189a4c

Please sign in to comment.