Skip to content
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

Monorepo - Cross reference to other package not resolved #2416

Closed
theovanderdonk opened this issue Oct 22, 2023 · 11 comments
Closed

Monorepo - Cross reference to other package not resolved #2416

theovanderdonk opened this issue Oct 22, 2023 · 11 comments
Labels
bug Functionality does not match expectation

Comments

@theovanderdonk
Copy link

Search terms

Cross reference other package not resolved

Expected Behavior

When I have a monorepo with multpple packages, and I create typedoc via "packages", I expect that types defined in one package but used in another package are properly linked from the other package to the one package.

Actual Behavior

These types do not become links, but they are plain text.

Steps to reproduce the bug

See TypeStrong/typedoc-repros#32
The readme contains repro steps + some additional observations.

Environment

  • Typedoc version: 25
  • TypeScript version: 4.9.5
  • Node.js version: 18.13.0
  • OS: Windows
@theovanderdonk theovanderdonk added the bug Functionality does not match expectation label Oct 22, 2023
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Oct 22, 2023

The code for inherited links predates all of the work for packages mode, and it looks like I missed it when implementing that feature... I'm not seeing an immediately obvious reason why I can't switch it to use symbol references now and just see it work.

Method parameters, property types, etc. should all just work.

@theovanderdonk
Copy link
Author

Thank you for your reply. I do not quite understand your wording. Do you mean you will investigate and try to switch to symboil references for my use cases? Or can/should I do that locally?

For method parameters, that also does not work. I have updated the example. So it appears that (at least for me) they do not 'just work'.

By the way -- compliments for the many improvements of TypeDoc over the last year or so. It is really looking much better, and monorepo also works very nice. It's just these little things I observe now :D

@theovanderdonk
Copy link
Author

Duplicate of / closely related to new issue #2420

@personalizedrefrigerator

For me, cross-project links work if packages have "types" point to a .ts file and not to a .d.ts file. See sample project.

@personalizedrefrigerator

I'm currently working around this with a custom unknownSymbolResolver. It's very fragile, but it mostly works.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Oct 29, 2023

Short story: You can get the behavior you want by turning on the declarationMap compiler option, and recompiling. In 0.25.3, you shouldn't need this compiler option anymore.

Looking at all three of #2428, #2420, #2416 when not on antihistamine makes me really concerned with anything I did last week... TypeDoc already correctly creates inheritance links with cross-references as it should, so my previous statement was just flat wrong.

To determine how to link files between packages, TypeDoc needs to be able to determine what file they'll be defined in. In 0.25.2, this is completely reliant on the existence of declaration maps, which seems to only be documented in a passing mention in the beta release issue for v0.24... However, when running in packages mode, there's really no reason that TypeDoc shouldn't be able to tell where files come from for the converted packages, since we have the compiler options and the file names which will be compiled.

I'm making an update to TypeDoc which will use this to derive source file names. I was hopeful that doing this would remove the requirement to build all the packages before running TypeDoc on them, but unfortunately this isn't quite enough since TypeScript (at least with a references based monorepo) requires that sub-projects be built. If emit is set to both when converting earlier projects, and projects are passed to TypeDoc in the order that they need to be built, TypeDoc's emit can be used rather than a pre-build, but it's easier to tell people to just build with tsc first, particularly since TypeDoc doesn't do any project dependency tracking.

I'm currently working around this with a custom unknownSymbolResolver. It's very fragile, but it mostly works.

I am surprised that works at all, TypeDoc was supposed to handle escaping, but it looks like I mistakenly assumed that \\" wouldn't close an attribute... I'm going to fix that now...

@theovanderdonk
Copy link
Author

Hi @Gerrit0 , For me it still does not work.

  • For 0.25.3 without declarationMap, it does not work. (provided that I first deleted my lib folder in which old declaration maps were still present. But with clean lib folders, after building both projects and then running typedoc, the links do not work)
  • For 0.25.3, it works when I enable declarationMap. But, you say that I should not need that setting.

In fact, I do not mind enabling this setting as this also has benefits for VSCode. But it is not in line with your remarks.

This aside, the "forward links" still do not work:

  • In the comments, my IBase has references to Sub and ISub, both of them are not resolved (plain text, not links)
  • The base interface IBase does not include Sub and ISub as implementing/deriving classes/interfaces. I understand they are in another package (and we cannot scan the entire world for all occurrances of a base class/interface), but they are in the same monorepo for a reason. I really would like to see them listed in the base class/interface documentation. (Alternatively, I can create manual links to them, but as described in the previous bullet, they also do not work).

@theovanderdonk
Copy link
Author

Update: A "forward link" can be generated like {@link mypackage!MyClass} but then it is also displayed as "myPackage!MyClass" which is simply ugly. Despite the setting for preserveLinkText, this does not seem like something someone would like to see.

@Gerrit0 Gerrit0 reopened this Nov 11, 2023
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Nov 11, 2023

I suspect the declarationMap issue not working is because I forgot to test on Windows, and paths are annoying...

The inheritance hierarchy is interesting, the way that's built today has always felt fragile to me... I'm going to look at improving it in a way which should also solve the cross-package inheritance issue.

Gerrit0 added a commit that referenced this issue Nov 26, 2023
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Nov 26, 2023

Smarter hierarchy detection isn't going to happen today... but at least I got to the windows path fix!

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Jan 1, 2024

I've decided to pull smarter hierarchy detection out into #2467 to avoid having to read through this issue every month or so when I forget what the actual remaining work for it is. I believe this is the only open problem still.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Functionality does not match expectation
Projects
None yet
Development

No branches or pull requests

3 participants