Skip to content

Commit

Permalink
Fix handling XRefs for Reference types that refer to a SymbolId (#132)
Browse files Browse the repository at this point in the history
I don't exactly understand how ReferenceTypes work in typedoc but they seem to
have two options:

1. The referent is a documented object and so there is a `reflection` field that
   points to the documentation reflection for the target.
2. It is not a documented object so the target is a SymbolId. This contains some
   information about the original typescript symbol, but not all of it.

So if we see a Reference it should definitely point to an external XRef. But
sometimes it seems to be that the SymbolId is also an internal object,
particularly when it's something being reexported.

This reference rendering code is still not quite right but at least now it
passes the added test.
  • Loading branch information
hoodmane authored May 3, 2024
1 parent a945112 commit b45ce77
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 24 deletions.
2 changes: 1 addition & 1 deletion sphinx_js/js/convertTopLevel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
import { sep, relative } from "path";
import { SphinxJsConfig } from "./sphinxJsConfig.ts";

function parseFilePath(path: string, base_dir: string): string[] {
export function parseFilePath(path: string, base_dir: string): string[] {
// First we want to know if path is under base_dir.
// Get directions from base_dir to the path
const rel = relative(base_dir, path);
Expand Down
55 changes: 33 additions & 22 deletions sphinx_js/js/renderType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
TypeXRefInternal,
intrinsicType,
} from "./ir.ts";
import { parseFilePath } from "./convertTopLevel.ts";

/**
* Render types into a list of strings and XRefs.
Expand Down Expand Up @@ -151,33 +152,43 @@ class TypeRenderer implements TypeVisitor<Type> {
// parameter.
return this.addTypeParams(type, [type.name]);
}
// TODO: should we pass down app.serializer? app?
const fakeSerializer = { projectRoot: this.basePath } as Serializer;
// Calling toObject resolves the file names with respect to projectRoot.
// qualifiedName and sourcefilename are supposed to be absolute.
const fileInfo = type.symbolId?.toObject(fakeSerializer);
// If it has a package field, it's external otherwise it's internal.
if (type.package) {
const res: TypeXRefExternal = {
if (type.reflection) {
const path = this.reflToPath.get(
type.reflection as DeclarationReflection,
);
if (!path) {
throw new Error(
`Broken internal xref to ${type.reflection?.toStringHierarchy()}`,
);
}
const res: TypeXRefInternal = {
name: type.name,
package: type.package,
qualifiedName: fileInfo?.qualifiedName || null,
sourcefilename: fileInfo?.sourceFileName || null,
type: "external",
path,
type: "internal",
};
return this.addTypeParams(type, [res]);
}
const path = this.reflToPath.get(type.reflection as DeclarationReflection);
if (!path) {
throw new Error(
`Broken internal xref to ${type.reflection?.toStringHierarchy()}`,
);
if (!type.symbolId) {
throw new Error("This should not happen");
}
const path = parseFilePath(type.symbolId.fileName, this.basePath);
let res: TypeXRefExternal | TypeXRefInternal;
if (path.includes("node_modules/")) {
// External reference
res = {
name: type.name,
package: type.package!,
qualifiedName: type.symbolId.qualifiedName || null,
sourcefilename: type.symbolId.fileName || null,
type: "external",
};
} else {
res = {
name: type.name,
path,
type: "internal",
};
}
const res: TypeXRefInternal = {
name: type.name,
path,
type: "internal",
};
return this.addTypeParams(type, [res]);
}
reflection(type: ReflectionType): Type {
Expand Down
11 changes: 11 additions & 0 deletions tests/test_typedoc_analysis/source/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Basic types: https://www.typescriptlang.org/docs/handbook/basic-types.html

import { Blah } from "./exports";

export enum Color {
Red = 1,
Green = 2,
Expand Down Expand Up @@ -115,8 +117,17 @@ export class ParamClass<S extends number[]> {

// Utility types (https://www.typescriptlang.org/docs/handbook/utility-types.html)

// Most type references in here have ResolvedReferences. These test cases are
// for "SymbolReference". See typedoc/src/lib/models/types.ts

// Partial should generate a SymbolReference that we turn into a
// TypeXRefExternal
export let partial: Partial<string>;

// Blah should generate a SymbolReference that we turn into a
// TypeXRefInternal
export let internalSymbolReference: Blah;

// Complex: nested nightmares that show our ability to handle compound typing constructs

export function objProps(
Expand Down
16 changes: 15 additions & 1 deletion tests/test_typedoc_analysis/test_typedoc_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,11 @@ def test_constrained_by_constructor(self):
assert join_type(obj.params[0].type) == "{new () => T}"

def test_utility_types(self):
"""Test that a representative one of TS's utility types renders."""
"""Test that a representative one of TS's utility types renders.
Partial should generate a SymbolReference that we turn into a
TypeXRefExternal
"""
obj = self.analyzer.get_object(["partial"])
t = deepcopy(obj.type)
t[0].sourcefilename = "xxx"
Expand All @@ -535,6 +539,16 @@ def test_utility_types(self):
">",
]

def test_internal_symbol_reference(self):
"""
Blah should generate a SymbolReference that we turn into a
TypeXRefInternal
"""
obj = self.analyzer.get_object(["internalSymbolReference"])
assert obj.type == [
TypeXRefInternal(name="Blah", path=["./", "exports"], type="internal")
]

def test_constrained_by_property(self):
obj = self.analyzer.get_object(["objProps"])
assert obj.params[0].type == [
Expand Down

0 comments on commit b45ce77

Please sign in to comment.