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

TypeDoc reports wrong class as parent class #1209

Closed
1 task done
benmccann opened this issue Feb 16, 2020 · 8 comments
Closed
1 task done

TypeDoc reports wrong class as parent class #1209

benmccann opened this issue Feb 16, 2020 · 8 comments
Labels
bug Functionality does not match expectation js This issue relates to better TS-in-JS support

Comments

@benmccann
Copy link

benmccann commented Feb 16, 2020

Expected Behavior

Should correctly determine parent class.

Actual Behavior

Says:

Overrides BubbleController.constructor
Defined in controllers/controller.line.js:27

When it should say

Overrides DatasetController.constructor
Defined in controllers/controller.line.js:27

Here's the code: https://github.com/chartjs/Chart.js/blob/7397a41fac0e9a57c969148eddc2805c8b249ee7/src/controllers/controller.line.js#L27

Steps to reproduce the bug

git clone [email protected]:chartjs/Chart.js.git
cd Chart.js
git reset --hard 7397a41fac0e9a57c969148eddc2805c8b249ee7
npm install
npx typedoc

Now open:
file:///directoryWhereYouCheckedOutTheCode/Chart.js/dist/docs/typedoc/classes/_controllers_controller_line_.linecontroller.html

Environment

  • Typedoc version: 0.16.9
  • Node.js version: 12.13.1
  • OS: Ubuntu
@benmccann benmccann added the bug Functionality does not match expectation label Feb 16, 2020
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Feb 17, 2020

That's... really weird. The hierarchy is correct, so TypeDoc did know what the parent class was, but copied in methods from a child class, which doesn't make sense.

Looks like this is only set in mergeDeclarations, so it must be called inappropriately somewhere. I'll take a closer look next weekend.

function mergeDeclarations(context: Context, reflection: DeclarationReflection, node: ts.Node, kind: ReflectionKind) {

@CoreyDotCom
Copy link

This issue still persists in latest TypeDoc... any workarounds??

@CoreyDotCom
Copy link

CoreyDotCom commented Aug 6, 2020

cc @Gerrit0 FWIW this was working fine in version 0.15.6 it seems. Can't use that version due to it depending on an older typescript though. Tested further and broke between 0.15.8 and 0.16.0-0.

@Gerrit0 Gerrit0 added the js This issue relates to better TS-in-JS support label Nov 25, 2020
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Nov 25, 2020

So... I've fixed this, for TypeScript projects. Unfortunately, it appears that work done to fix some other issues with inheritance breaks running TypeDoc on Chart.js entirely since we don't have special casing for all of the JSDoc style types. I'm sorry to say that I don't plan on fixing this - at least initially for 0.20. I'd like to support JS projects too, but the primary focus of TypeDoc is TypeScript projects.

For this to be supported:

  • Add a special case to convertTypeAlias in src/lib/converter/symbols.ts for JSDocTypedef nodes
  • Probably also add a bunch of type converters to src/lib/converter/types.ts for JSDoc type nodes

@donmccurdy
Copy link

@Gerrit0 I've been tracking this issue because it appears to be affecting my (TypeScript) project. Are you saying that this bug is specific to JS projects? Or that it was affecting all projects, but has since been fixed for TS projects and not JS projects? If the latter, what version of typedoc should the fix be available in? Thanks!

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Nov 27, 2020

It was affecting all projects, but 0.20 contains a fix for TS projects. It likely also fixes the issue in JS projects, but I was unable to confirm this with the Chart.js test case since it uses JSDoc style types. 0.20 is released as a beta now, and will be released fully before the end of the year.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Jan 3, 2021

I just double checked, and confirmed that this is fixed in 0.20.9 for Chart.js

@Gerrit0 Gerrit0 closed this as completed Jan 3, 2021
@benmccann
Copy link
Author

That's awesome! Thanks for the amazing 0.20 release and all your help!!

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 js This issue relates to better TS-in-JS support
Projects
None yet
Development

No branches or pull requests

4 participants