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

fix: Resolve the name of parameters referring to default exports #2574

Merged
merged 2 commits into from
May 24, 2024

Conversation

tristanzander
Copy link
Contributor

@tristanzander tristanzander commented May 15, 2024

Hello! My team recently discovered some strange behavior with TypeDoc resolving the type name of a class as default instead of the name of the class. I discovered that this happens when the following constraints are met:

  1. A file that's not included in the documentation exports a default function/class.
  2. Another file exports that default export as a non-default member.
  3. A file that is included in documentation references that type in the parameter of a function, a property of a class, etc.

Note: This can only happen if the function/class is declared using export default class or export default function, since TypeScript will resolve the type back to any normal export class or export function with the correct name.

I noticed that TypeScript itself resolves the parameter's symbol type as the name default instead of the name of the type. I'm unsure if this is correct, so I applied conditional logic to help handle this particular case.

Here's a visual example from our generated docs:
image
...where the correct type should be DataTable.

Please let me know if a better solution is more desirable 🙏

src/test/issues.c2.test.ts Outdated Show resolved Hide resolved
src/lib/models/types.ts Outdated Show resolved Hide resolved
src/test/issues.c2.test.ts Outdated Show resolved Hide resolved
src/test/issues.c2.test.ts Outdated Show resolved Hide resolved
@tristanzander tristanzander force-pushed the master branch 2 times, most recently from 0cdcd29 to 6a928dc Compare May 16, 2024 16:59
@tristanzander tristanzander requested a review from Gerrit0 May 16, 2024 16:59
@tristanzander
Copy link
Contributor Author

@Gerrit0 I tried to find a way to resolve those symbols as their re-exported name but I couldn't find I decent solution that didn't break other tests. This seems to be the way that type checker wants to resolve those types. Do you have any ideas for how I would determine if a given type was re-exported to apply this logic?

@tristanzander
Copy link
Contributor Author

tristanzander commented May 16, 2024

I am noticing that the declaration.type.id and type.id are different in this example. That might be a way to determine if the types are different, but I'm not sure how generics come into play here (that's where most of the other tests seemed to break on)

@Gerrit0
Copy link
Collaborator

Gerrit0 commented May 16, 2024

I incorrectly assumed that making the change to derive the name in the type converter would just make that work - given that the type converter is getting the resolved type, I think it's reasonable to leave the behavior as is

@tristanzander
Copy link
Contributor Author

@Gerrit0 Any chance I could get a re-review on this soon?

@Gerrit0
Copy link
Collaborator

Gerrit0 commented May 21, 2024

I meant to merge it last weekend, unfortunately it fell off my list. I'll definitely get it in next weekend!

@Gerrit0 Gerrit0 merged commit 33a34d6 into TypeStrong:master May 24, 2024
4 checks passed
@Gerrit0
Copy link
Collaborator

Gerrit0 commented May 24, 2024

Thanks!

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