Strange behavior in complicated hybrid (esm/cjs) workspaces doc generation

[isaacs]

Search terms

• workspaces
• hybrid
• cjs
• esm
• tsconfig
• references

Expected Behavior

A bit of context at first. This is a somewhat complicated scenario and frankly, I'm surprised it works at all, so I'm quite impressed with TypeDoc's ability to handle it as well as it does.

Repo: https://github.com/tapjs/tapjs
Docs output: https://tapjs.github.io/tapjs/

The project is a collection of workspace packages, almost all of which are dual-built as ESM and CJS into separate folders, usually dist/cjs for commonjs, and dist/mjs for ESM. Example tsconfig for the esm build:

• tsconfig/esm.json
• tsconfig/build.json
• tsconfig.json

The @tapjs/test package at ./src/test has a generated implementation class, which extends the TestBase class defined in @tapjs/core at src/core/src/test-base.ts. That Test class is in turn extended by the TAP class in @tapjs/core, at src/core/src/tap.ts.

So there's quite a bit of weird cycling and bootstrap problems, but the end result is dynamic types that work based on configurable plugins, and also are fully statically analyzable, so tsserver can provide the right code hints including all the plugins when writing tests. In order to get it to build properly and get around the bootstrap issues, the @tapjs/core and @tapjs/test keep their builds checked into git, and there's a script that will bootstrap the whole thing from a fresh checkout via npm run bootstrap.

I'm attempting to generate documentation for all of the workspaces in the project. My root tsconfig.json has a reference to all the workspaces. In each workspace, I have a typedoc.json which I think should be telling it to use the tsconfig/esm.json configuration, and only include entry point files that are built as esm.

My expectations are that it should effectively document the entire project as if it was ESM-only, and be able to follow the references using entirely {"type":"module"} style import() resolution.

Actual Behavior

• Many references raise warnings that they are not found, and the warning refers to the cjs paths. For example, while generating docs for src/core, it prints these for nearly everything in the TAP class:

[warning] Base, defined in ./src/core/dist/cjs/base.d.ts, is referenced by
tap.TAP.activeSubtests but not included in the documentation.

The confusing thing is that it's looking in dist/cjs, rather than dist/mjs, even though the tsconfig in effect should be such that this module would only ever load the esm versions.

• Some {@link} references are just not resolved, but only in some cases. For example, in the comment for the definition of the TAP.timeout method, the link to TestBase#timeout is not resolved. However, when the class is re-exported as part of the main tap package, it does resolve.

I've tried specifying the links with the full namespaced package and module path, like {@link @tapjs/core.test-base.TestBase#timeout}, or without the module name like {@link @tapjs/core.TestBase#timeout} since that seems to be allowed by the tsdoc specs, but it seems like anything I add just ends up never resolving at all.

Steps to reproduce the bug

I've tried creating a more minimal example to reproduce this bug, but I haven't yet been able to. I think it's likely that I'm just finding some edge cases that only happen when you have a combination of cyclic references, hybrid builds, and interlinked workspaces.

Any clues or thoughts would be much appreciated, even if this is just a case that typedoc shouldn't be expected to handle. It would be nice to be able to leverage typedoc as much as possible in tap's API docs, but I'm starting to wonder if the answer might be to just have typedoc output json, and then build up the site using that as a data source somehow.

Environment

• Typedoc version: 0.24.8
• TypeScript version: 5.1.3
• Node.js version: v18.16.0
• OS: same behavior on macOS and ubuntu

[isaacs]

Other declaration reference styles I tried that also don't work:

• Package name, file, and export name separated by dot: @tapjs/mock.index.TapMock
• Package name, file, and export name separated by #: @tapjs/mock.index#TapMock
• Package name, and the export separated by #: @tapjs/mock#TapMock

In all cases, the link created has a href set to the current module, and the text is not condensed to show just the last bit of the reference, like it does with declaration references it resolves successfully.

[Gerrit0]

I should be able to take a look this weekend - at first glance, looks like your configuration ought to work...

[isaacs]

Sweet. I'm in the process of migrating everything to be esm-by-default (ie, putting "module":"esnext" in the base tsconfig, and "type":"module" in the package.json, and migrating tests to esm.)

That corrects the "defined in" links, which suggests to me that it's either not using the tsconfig I'm telling it to use (seems less likely), or it's looking up the imported dep using require.resolve instead of import resolution. If it's the second one, and it's your code calling require.resolve, then the resolve-import module on npm might be worth looking at.

[Gerrit0]

The only places in TypeDoc where require.resolve is used is for "extends" processing in typedoc.json/tsconfig.json, when resolving entry points with legacy-packages mode, and for getting the path to TypeScript for debug prints, so it isn't that...

Do I need to do something special to get a working install? On 3bfcd96, with a fresh clone and npm i, node 18.17.0, npm 9.8.1

npm ERR! code 2
npm ERR! path /home/gerrit/Desktop/tapjs/src/create-plugin
npm ERR! command failed
npm ERR! command sh -c tsc -p tsconfig/esm.json && bash scripts/fixup.sh
npm ERR! src/index.ts(4,22): error TS2307: Cannot find module 'npm-init-template' or its corresponding type declarations.

npm ERR! A complete log of this run can be found in: /home/gerrit/.npm/_logs/2023-08-13T18_59_37_875Z-debug-0.log
exit 2

I really don't like prepare scripts...

[isaacs]

Yeah, run npm run bootstrap.

The issue is that I have some "impossible" circular links in order to support the code generated Test class without typescript getting confused.

[Dayday10]

Hum

…

On Sun, Aug 13, 2023, 2:39 PM isaacs ***@***.***> wrote: Yeah, run npm run bootstrap. The issue is that I have some "impossible" circular links in order to support the code generated Test class without typescript getting confused. — Reply to this email directly, view it on GitHub <#2360 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AWMIKFNYINNLIGGC6TFUBYLXVEUQBANCNFSM6AAAAAA3GJIGEY> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[Gerrit0]

Some {@link} references are just not resolved, but only in some cases

Turns out this one was just that I forgot to handle qualified names when asking TS to resolve a link. {@link @tapjs/core.test-base.TestBase#timeout} works with TypeDoc's custom resolution for me. With this fixed, I only get the following warnings for broken links:

[warning] Failed to resolve link to "Test" in comment for @tapjs/after.
[warning] Failed to resolve link to "Test" in comment for @tapjs/after-each.
[warning] Failed to resolve link to "Test" in comment for @tapjs/before.
[warning] Failed to resolve link to "Test" in comment for @tapjs/before-each.
[warning] Failed to resolve link to "TAP" in comment for @tapjs/core.base.Base.oncomplete.
[warning] Failed to resolve link to "TAP" in comment for @tapjs/core.minimal.Minimal.oncomplete.
[warning] Failed to resolve link to "TAP" in comment for @tapjs/core.spawn.Spawn.oncomplete.
[warning] Failed to resolve link to "TAP" in comment for @tapjs/core.stdin.Stdin.oncomplete.
[warning] Failed to resolve link to "Test" in comment for @tapjs/core.tap.
[warning] Failed to resolve link to "TAP" in comment for @tapjs/core.test-base.TestBase.oncomplete.
[warning] Failed to resolve link to "TAP" in comment for @tapjs/core.test-base.TestBaseEvents.idle.
[warning] Failed to resolve link to "TestBase#waitOn | t.waitOn" in comment for @tapjs/core.waiter.Waiter.
[warning] Failed to resolve link to "TestBase#queue" in comment for @tapjs/core.waiter.Waiter.
[warning] Failed to resolve link to "TestBase" in comment for @tapjs/core.waiter.Waiter.finish.
[warning] Failed to resolve link to "TAP" in comment for @tapjs/core.worker.Worker.oncomplete.
[warning] Failed to resolve link to "TAP" in comment for @tapjs/core.worker.WorkerOpts.workerData.
[warning] Failed to resolve link to "defaultMaxListeners" in comment for @tapjs/nock.recorder.ScopeWithAt.getMaxListeners.
[warning] Failed to resolve link to "Has" in comment for tcompare.same.Same.
[warning] Failed to resolve link to "TAP" in comment for @tapjs/test.index.Test.
[warning] Failed to resolve link to "TAP" in comment for @tapjs/test.index.Test.oncomplete.
[warning] Failed to resolve link to "Extra" in comment for @tapjs/test.index.TestOpts.
[warning] Failed to resolve link to "BaseOpts" in comment for @tapjs/test.index.TestOpts.

If TypeScript can't resolve a link, TypeDoc's link resolution works by searching the current "scope" for a reflection matching the name, then going up to the parent scope and checking that one... so taking the TestBase link in Waiter as an example, TypeDoc looks for a member called TestBase in Waiter, doesn't find one, so looks for a member in waiter, then in @tapjs/core, then in the root project. Since TestBase is at <root>.@tapjs/core.test-base.TestBase, there are several ways to write this link:

test-base.TestBase#waitOn
@tapjs/core.test-base.TestBase#waitOn
@tapjs/core!test-base.TestBase.waitOn

The declaration references page should probably have a couple examples of this to make this clearer...

Many references raise warnings that they are not found, and the warning refers to the cjs paths. For example, while generating docs for src/core, it prints these for nearly everything in the TAP class:

The warnings I'm seeing on 3bfcd960deebf6e2235d5093c998e1d0a8d1b0b3 are printing esm paths, I took a look at the code that prints those declaration warnings, however, and I'm getting that path directly from TypeScript, so however your project was set up before, it was actually using the cjs types.

[isaacs]

The warnings I'm seeing on 3bfcd960deebf6e2235d5093c998e1d0a8d1b0b3 are printing esm paths, I took a look at the code that prints those declaration warnings, however, and I'm getting that path directly from TypeScript, so however your project was set up before, it was actually using the cjs types.

The only thing I can think of is that the root tsconfig.json didn't have "module": "esnext", so even though the workspace package actually being considered should have been using esm (by virtue of the "tsconfig":"./tsconfig/esm.json" option in typedoc.json), it thought it was using cjs? Before changing that, I did verify that the code was pulling in the definitions from the relevant dist/mjs folder by deleting it dist/cjs, and verifying that tsc -p tsconfig/esm.json could still build properly.

there are several ways to write this link:

Using the latest master branch of TypeStrong/typedoc, when I use {@link @tapjs/core.test-base.TestBase.waitOn}, in src/core/src/waiter.ts, I still get the incorrect value of:

<a href="../modules/_tapjs_core.waiter.html" class="tsd-kind-module">@tapjs/core.test-base.TestBase.waitOn</a>

But, using {@link @tapjs/core!test-base.TestBase#waitOn} (with an exclamation point), I get the expected:

<a href="_tapjs_core.test_base.TestBase.html#waitOn" class="tsd-kind-method">waitOn</a>

[Gerrit0]

The only thing I can think of is that the root tsconfig.json didn't have "module": "esnext"

I commented that out, and also experimented with removing the "type": "module" from the root package.json with no luck - what do I need to change/checkout to in order to see the CJS paths?

Using the latest master branch of TypeStrong/typedoc, when I use {@link @tapjs/core.test-base.TestBase.waitOn}, in src/core/src/waiter.ts, I still get the incorrect value of:

Sorry for the misdirection! I had forgotten that @ is not a valid character by itself according to the declaration reference grammar:

https://github.com/microsoft/tsdoc/blob/fcffaae1e7c0b8797174f77519436a6e6b8b312f/tsdoc/src/beta/DeclarationReference.grammarkdown#L44-L45

https://github.com/microsoft/tsdoc/blob/fcffaae1e7c0b8797174f77519436a6e6b8b312f/tsdoc/src/beta/DeclarationReference.grammarkdown#L129-L130

{@link "@tapjs/core".test-base.TestBase.waitOn} would work. There's a second bug there in that TypeDoc really should have not resolved this link at all rather than linking to an obviously unintended page. I've added some code to suggest a change to ! if TypeDoc notices a link like this, and also fixed the bad linking.

[Gerrit0]

Closing this as I believe we've worked through most of the issues, and I was never able to reproduce the CJS paths one