Skip to content

Ability to define a converter for JSDocNamepathType #2947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pomek opened this issue Apr 28, 2025 · 6 comments
Closed

Ability to define a converter for JSDocNamepathType #2947

pomek opened this issue Apr 28, 2025 · 6 comments

Comments

@pomek
Copy link

pomek commented Apr 28, 2025

Search Terms

  • JSDocNamepathType

Problem

The problem I described touches the 0.28 branch. However, to give you some context, I'd like to share a brief story.

In CKEditor 5, we render API pages based on output produced by typedoc. We wrote a custom plugin for handling the @error tag, so the following code (block comment) - https://github.com/ckeditor/ckeditor5/blob/dcb0b8c936c197acf215a96153350d0a4ce83834/packages/ckeditor5-core/src/accessibility.ts#L331-L339 - can be displayed on our error codes page.

Recently, we started migrating from version 0.23. x to the latest (0.28.3 at the moment of writing this). The documentation is based on the older version so far.

While processing the project, typedoc prints a few warnings that look like this:

./packages/ckeditor5-core/src/accessibility.ts:338:14 - [warning] Failed to convert type node with kind: JSDocNamepathType and text module:core/accessibility~AddKeystrokeInfosData#keystrokes. Please report a bug.

338                      * @param {module:core/accessibility~AddKeystrokeInfosData#keystrokes} keystrokes Keystroke definitions about to be added.

Even when the warning appears, I can translate the {module:...} part to a link because I know how to map a module between brackets.

Image

☝ This is a screenshot of my local build using the latest typedoc package and the plugin I share below.

There is a logic responsible for converting parameters from the @param annotation for error codes: https://github.com/ckeditor/ckeditor5-dev/blob/4f1a3acfd3b79c634b9b5b147c54ce922b61bfc7/packages/typedoc-plugins/src/tag-error/index.ts#L133-L158

I would like to request the addition of an option to define a converter for resolving the JSDocNamepathType kind. Or, at least, to mute the warnings, because in my case, they are false positives.

@pomek
Copy link
Author

pomek commented Apr 28, 2025

I believe I would like to add something here:

export function loadConverters() {
if (converters.size) return;
for (
const actor of [
arrayConverter,
conditionalConverter,
constructorConverter,
exprWithTypeArgsConverter,
functionTypeConverter,
importType,
indexedAccessConverter,
inferredConverter,
intersectionConverter,
intrinsicConverter,
jsDocVariadicTypeConverter,
keywordConverter,
optionalConverter,
parensConverter,
predicateConverter,
queryConverter,
typeLiteralConverter,
referenceConverter,
restConverter,
namedTupleMemberConverter,
mappedConverter,
literalTypeConverter,
templateLiteralConverter,
thisConverter,
tupleConverter,
typeOperatorConverter,
unionConverter,
jSDocTypeExpressionConverter,
// Only used if skipLibCheck: true
jsDocNullableTypeConverter,
jsDocNonNullableTypeConverter,
]
) {
for (const key of actor.kind) {
if (key === undefined) {
// Might happen if running on an older TS version.
continue;
}
assert(!converters.has(key));
converters.set(key, actor);
}
}
}

The current implementation does not permit this.

@pomek
Copy link
Author

pomek commented Apr 28, 2025

AI-based solutions try to something like this:

app.converter.on(Converter.EVENT_CREATE_DECLARATION, (context, reflection, node) => {
    if (node?.kind === ts.SyntaxKind.JSDocNamepathType) {
        // Create a custom type manually
        const myType = new Type('custom-jsdoc-namepath');

        reflection.type = myType;
    }
});

// Alternatively, a lower-level hook could be added:
app.converter.addUnknownNodeConverter({
    supports: (node) => node.kind === ts.SyntaxKind.JSDocNamepathType,
    convert: (context, node) => {
        // You would resolve the namepath here.
        return new ReferenceType('EditorConfig', context.project);
    },
});

or even more hacky:

app.converter.on('resolveBegin', (context: Context) => {
  const typeConverter = context.converter.typeConverter;

  const oldConvertNode = typeConverter.convertNode.bind(typeConverter);

  typeConverter.convertNode = function (context, node) {
    if (node.kind === ts.SyntaxKind.JSDocNamepathType) {
      const text = (node as any).name.getText(); // risky, but no better option
      return new ReferenceType(text, context.project);
    }

    return oldConvertNode(context, node);
  };
});

But none of this works.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Apr 29, 2025

TypeDoc's converters are not extensible today because the discrimination logic is considered an implementation detail and is subject to change at any time... I'm not interested in exposing hooks for that at this time as I'm not convinced the existing method of discrimination is a good way of doing it. It mostly works, but has several annoying edge cases.

If JSDocNamepathType is a type which TypeDoc can be caused to convert without a plugin, it is a bug that TypeDoc doesn't handle it today. However, I suspect this isn't the case because TypeDoc will convert the JSDocNamepathType.type property instead, which is a regular type node instead of a JSDoc-hack that will break with ts-go anyways (namepath support is being dropped). Your plugin should be able to pass that to have TypeDoc convert it as a type reference today.

AI is utterly hopeless at doing anything even remotely useful in TypeDoc's source.

@pomek
Copy link
Author

pomek commented Apr 29, 2025

Thanks for the answer. So, let's go back to this warning:

./packages/ckeditor5-core/src/accessibility.ts:338:14 - [warning] Failed to convert type node with kind: JSDocNamepathType and text module:core/accessibility~AddKeystrokeInfosData#keystrokes. Please report a bug.

338                      * @param {module:core/accessibility~AddKeystrokeInfosData#keystrokes} keystrokes Keystroke definitions about to be added.

How could I suppress it? I want to avoid modifying the logger level because we have several plugins that also print warnings. Switching them to the error level does not make sense as we treat warnings as something to fix while it does not crash.

@pomek
Copy link
Author

pomek commented Apr 29, 2025

Self-answer: avoid executing context.converter.convertType when a text between brackets starts with module:. It would still fail when we expect an array or union, but this can be easily worked around by defining a custom type that is used in this context.

So, I think, we can close the issue.

@pomek pomek closed this as completed Apr 29, 2025
@Gerrit0
Copy link
Collaborator

Gerrit0 commented May 1, 2025

@pomek #2949 implies that it should be possible to reproduce this without a plugin, which would make it in scope for a TypeDoc bug... I'm unfamiliar with this type, and a few attempts didn't work, are you able to supply a single file reproduction which demonstrates how to use them?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants