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

Don't warn for excluded external links in comments #2855

Closed
wants to merge 1 commit into from

Conversation

pjeby
Copy link
Contributor

@pjeby pjeby commented Feb 15, 2025

Warnings are still generated for non-working links in non-comment sources, since those won't be resolved by VSCode. But comment links aren't warned about if externals are excluded. (#2853)

(I'm not sure whether this is the proper solution, my view of things is way too narrow. So any feedback you can offer here would be great.)

Warnings are still generated for non-working links in
non-comment sources, since those won't be resolved by
VSCode.
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Feb 15, 2025

This isn't quite the right way to approach this as it assumes that excludeExternals is the only reason we might have a symbol ID for something, but not be able to link to it. A few more are:

  • The symbol was marked with @hidden
  • The symbol isn't exported, so isn't included in the docs
  • A plugin removed the symbol (e.g. typedoc-plugin-no-inherit)
  • The symbol wasn't documented, and excludeNotDocumented was specified

Personally, if an @link isn't going to work in the documentation, I want to see warnings about it! Your use case of having it still work in VSCode is reasonable though. I think the right way to do this is to introduce a new option within the validation option, perhaps validation.excludedLink which defaults to true and causes warnings to be created if there is a ReflectionSymbolId for a link, but no corresponding reflection.

@pjeby
Copy link
Contributor Author

pjeby commented Feb 15, 2025

Personally, if an @link isn't going to work in the documentation, I want to see warnings about it!

That is also reasonable. It's just that there's no way to silence the warning about any particular link. If I could silence them one by one (without breaking the VSCode usability), that'd be fine, too.

I'd also like to have the exclusion be narrower than just "have a symbol ID but can't link", since some of those other scenarios you described are things I'd want to know about.

It's possible that what I should be doing is adding external symbol resolvers or something, but then I don't think there's a way to make that result in a non-link, or to reliably calculate an actual external link when it's to a symbol in another typedoc project.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Feb 15, 2025

I suppose we could pass the external patterns to the link validator, and name that option unresolvedExternalLink. typedoc-plugin-mdn-links for globals and typedoc-plugin-dt-links have taken care of most external links for me.

I don't think there's a way to make that result in a non-link

That's correct... I guess externalSymbolLinkMappings could be extended to recognize a special <ignore> string that causes the link not to be rendered, but also not reported... not a huge fan of that, but given that it needs to be configurable through a (potentially JSON) config file...

or to reliably calculate an actual external link when it's to a symbol in another typedoc project.

Not really at this point unfortunately. This is one of those features that's annoyingly difficult to get fully right, and to do fully would require fetching content from the doc site, which is something I'm very hesitant to do in TypeDoc as it breaks the current policy of no external requests. Maybe it's something I should experiment with in a plugin first... all the necessary API hooks are present.

@pjeby
Copy link
Contributor Author

pjeby commented Feb 16, 2025

So I played around with externalSymbolLinkMappings, and was able to quiet down most of the things that weren't re-exports, but it took adding a dozen URLs, and externalSymbolLinkMappings doesn't work right with modern module exports. (i.e., if you have a package foo with a foo/bar export, you have to list both bar's exports under foo as well - so best hope there aren't any names in common in your subpackages.)

I also ran into a related issue, which is that some of the warnings were due to re-exports: apparently if you re-export something external with members, typedoc looks at the comments on its members and tries to generate a page for them, which then results in more symbols needing to be looked up in the mappings. It would be nice if there was a way to say, "don't treat this re-export as a locally-defined thing, just link to its external link," if there's not one I'm ignorant of.

By the way, for context, the specific reason I've got so much of this is that I had a library that defined some rough versions of some things that I later spun out to a separate, more robust library, and the old library now has wraps the new library with deprecated shims over the old API. So there are lots of comments with @deprecated Use {@link thingFromLibrary} instead.

Now that I know this is an issue, I can probably avoid it by just not bothering to use links in the first place in future, or else be prepared to create lots of external link mappings. The re-export thing is still a bit of a challenge, though.

@pjeby
Copy link
Contributor Author

pjeby commented Feb 16, 2025

Oh, and regarding the linking to other projects, do note that you can get away without using requests if it can be done with a json export. Like if a project ships a typedocModuleLinks.json, it can be read from disk and it'll even handle versions correctly if someone's creating versioned builds. 😁

I would also suggest that the "ignore" could just be # (not falsy, but link gen can easily do url && url !=='#'), and
that entries in externalSymbolLinkMappings could be just "packageName": "#" as a way to disable the links entirely for that package.

But these are just brainstorming; for my own use case the dozen hand-rolled urls are taking care of the problem for now, and they'll go away when I drop the deprecated API shims. (At least until the next time I roll something out of one library into another.)

Anyway, I figure I should probably go ahead and close this PR since it doesn't really offer a starting point for any of these ideas or strategies. 😁

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Feb 16, 2025

if you have a package foo with a foo/bar export, you have to list both bar's exports under foo as well - so best hope there aren't any names in common in your subpackages

This should probably be fixed, it is similar to the type of change for #2833, so I'll probably wait for that first

apparently if you re-export something external with members, typedoc looks at the comments on its members and tries to generate a page for them

This ought to not generate a page if you have excludeExternals turned on...

Oh, and regarding the linking to other projects, do note that you can get away without using requests if it can be done with a json export. Like if a project ships a typedocModuleLinks.json, it can be read from disk and it'll even handle versions correctly if someone's creating versioned builds. 😁

Yeah, that's definitely a better fit, it's unfortunate to encourage npm package bloat, but compression should hopefully take care of most of it...

I would also suggest that the "ignore" could just be # (not falsy, but link gen can easily do url && url !=='#')

I like this better than what I came up with, falsy is already used for unsuccessful resolution.

@Gerrit0 Gerrit0 closed this in e61de01 Feb 21, 2025
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

Successfully merging this pull request may close these issues.

2 participants