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

addUnknownSymbolResolver to expose more than name #1832

Closed
jpoehnelt opened this issue Dec 30, 2021 · 2 comments
Closed

addUnknownSymbolResolver to expose more than name #1832

jpoehnelt opened this issue Dec 30, 2021 · 2 comments
Labels
bug Functionality does not match expectation enhancement Improved functionality

Comments

@jpoehnelt
Copy link

jpoehnelt commented Dec 30, 2021

Problem

addUnknownSymbolResolver accepts a resolver that receives ts.Symbol.name, however the name is insufficient to identify the symbol in cases where the symbol is namespaced such as the following:

declare namespace foo.bar {
  class Baz {}
}

in the resolver this is only identified as Baz and not foo.bar.Baz

Suggested Solution

1. allow the resolver to access ts.Symbol

app.renderer.addUnknownSymbolResolver("my-package", (symbol: Symbol) => {
  // get namespace from Symbol.parent
   return '';
 });

This would be a breaking change and may expose a more complex interface than needed(?).

2. pass the namespaced name joining ts.Symbol.parent resulting in foo.bar.Baz.

This option would not be a breaking change and is sufficient for my use case.

@jpoehnelt jpoehnelt added the enhancement Improved functionality label Dec 30, 2021
@Gerrit0 Gerrit0 added the bug Functionality does not match expectation label Jan 4, 2022
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Jan 4, 2022

ts.Symbol.parent is marked @internal for a reason, it's behavior isn't perfectly consistent in all cases (there's an issue about exposing it in Microsoft/TypeScript... somewhere where this was brought up)

I don't want to give resolvers access to the symbol since really they ought to be able to execute without access to the compiler (will be required if I ever get around to making it possible to generate docs from JSON, which is a rather important next step for making the packages entry point strategy usable for repos which don't release every package whenever one releases)

I've pushed a tentative fix for this, but want to look at it again this weekend before merging.

@Gerrit0 Gerrit0 closed this as completed in 52c8c4f Jan 8, 2022
@jpoehnelt
Copy link
Author

Thanks for the quick turnaround!

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 enhancement Improved functionality
Projects
None yet
Development

No branches or pull requests

2 participants