feat(typescript): class docs should link to inherited class docs #207

devversion · 2017-02-08T23:50:40Z

Introduces a new property on export docs that links to the class docs that are being inherited.

petebacondarwin · 2017-02-19T16:06:03Z

typescript/mocks/readTypeScriptModules/inheritedMembers.ts

+
+export class FirstParent extends LastParent {
+  firstParentProp: string = 'Works';
+  _privateProperty: string = 'Private';


you can also specify privacy by putting the private keyword before the property

We could. But I wanted this check to be private due to the leading underscore.

Perhaps we should test both styles of private?

Sounds good.

petebacondarwin · 2017-02-19T16:10:18Z

typescript/processors/readTypeScriptModules.js

+            var inheritedMembers = resolveInheritedMembers(resolvedExport, typeChecker);
+
+            for (var memberName in inheritedMembers) {
+              if (memberName === 'T') continue;


This seems rather hard coded. I guess you are trying to spot generic methods?
What if someone used a different letter in their generic definition?
Or am I missing something?

I didn't put much effort into that. This logic was copied from the for-loop below. Looks like Alex Eagle wrote it at this time.

I see...
OK well if it worked before then I guess we should leave it the same for consistency for now.
Perhaps we could factor that bit into a shared function so that we only have to change it in one place if we decide it needs fixing in the future?

petebacondarwin

This looks much better. I made a couple of inline comments.

I think it would be more effective if the inherited members each had a property that referenced the type from which they came. That way, one could group the inherited members by their heritage.

An alternative approach to this would be to identify the actual parent type (or more specifically the dgeni doc that represents the type) and just provide a property that references it. This way processors or templates could do the work of actually searching up the tree for the inherited members themselves - and you would get the ownership information I requested above for free.

* Introduces a new property on export docs that links to the class docs that are being inherited.

petebacondarwin · 2017-03-04T14:41:14Z

typescript/processors/readTypeScriptModules.js

+      function resolveInheritedDocs(exportSymbol, exportDoc) {
+        var inheritedSymbols = resolveInheritedSymbols(exportSymbol, typeChecker);
+        var inheritedDocs = inheritedSymbols.map(function(symbol) {
+          var relatedDoc = docs.find(function(doc) { return doc.exportSymbol === symbol; });


This could be super slow, given that we are doing a search of o(n) on every single inheritance symbol.

That's true. Should we use some faster search algorithms? Or do you have any better ideas? I thought about creating a Map of Symbols with their associated docs, but that would then fill-up memory..

petebacondarwin · 2017-03-04T14:50:00Z

OK, so this is really cool. I am worried about the performance time of doing all the searching.
Instead, can I suggest the following alternative approach:

in the readTypeScriptModules processor:
- only capture the inheritance symbols in each export doc. E.g. doc.inheritanceSymbols
- and add each exportDoc to a map keyed on its export symbol. E.g. `symbolToDocMap = new Map()
then in a new processor, which runs later, say, WireUpInheritanceDocs, you iterate through each export doc's inheritance symbols and map them to their appropriate doc

This way we do not have to keep searching the docs for the appropriate symbol.

What do you think?

(sorry this is turning into a bit of an epic!)

devversion · 2017-03-04T15:35:20Z

@petebacondarwin Made the requested changes. Please review again.

petebacondarwin · 2017-03-04T16:12:47Z

typescript/processors/linkInheritedDocs.js

+ **/
+module.exports = function linkInheritedDocs(typescriptSymbolMap) {
+  return {
+    $runAfter: ['readTypeScriptModules'],


it is probably worth putting in a $runBefore property too, since we can't guarantee that it will be run before other processors otherwise...

petebacondarwin · 2017-03-04T16:19:14Z

typescript/mocks/readTypeScriptModules/inheritedMembers.ts

+ * To ensure that Dgeni properly recognizes classes from exports that will be parsed later,
+ * the `FirstParent` class will be ordered after the `Child` class.
+ **/
+export class FirstParent extends LastParent {


is it worth putting in some implements clauses to see what would happen?

devversion · 2017-03-06T17:49:45Z

@petebacondarwin Ping for review :)

petebacondarwin · 2017-03-06T21:33:24Z

We got there!
Landed and published as 0.16.9
Thanks for your hard work and patience @devversion

Introduce a new property `doc.inheritedDocs` on export docs that links to the class docs that this doc inherited from. This can be used in templates to show members for a class that were inherited from other classes. Closes #207

googlebot added the cla: yes label Feb 8, 2017

mmalerba approved these changes Feb 9, 2017

View reviewed changes

devversion mentioned this pull request Feb 10, 2017

refactor: add themeable base class angular/components#3036

Closed

devversion force-pushed the fix/ts-inherit-members branch from c11fdaa to c0b1440 Compare February 18, 2017 11:20

petebacondarwin reviewed Feb 19, 2017

View reviewed changes

petebacondarwin suggested changes Feb 19, 2017

View reviewed changes

devversion force-pushed the fix/ts-inherit-members branch from e869492 to fc85b36 Compare February 28, 2017 15:23

devversion changed the title ~~fix: typescript should respect inherited members~~ feat(typescript): class docs should link to inherited class docs Feb 28, 2017

feat(typescript): class docs should link to inherited class docs

4c5b2cc

* Introduces a new property on export docs that links to the class docs that are being inherited.

devversion force-pushed the fix/ts-inherit-members branch from fc85b36 to 4c5b2cc Compare February 28, 2017 15:25

petebacondarwin reviewed Mar 4, 2017

View reviewed changes

Updates

99449c6

devversion force-pushed the fix/ts-inherit-members branch from 91ab3fa to 99449c6 Compare March 4, 2017 15:34

rename to TypeScript Symbol Map

a02738d

petebacondarwin reviewed Mar 4, 2017

View reviewed changes

devversion added 2 commits March 4, 2017 19:12

Add test interface

538e8cd

Run before parsing-tags

d97e1e6

petebacondarwin approved these changes Mar 6, 2017

View reviewed changes

petebacondarwin closed this in cf8b744 Mar 6, 2017

feat(typescript): class docs should link to inherited class docs #207

feat(typescript): class docs should link to inherited class docs #207

Uh oh!

Conversation

devversion commented Feb 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devversion Feb 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petebacondarwin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petebacondarwin commented Mar 4, 2017

Uh oh!

devversion commented Mar 4, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devversion commented Mar 6, 2017

Uh oh!

petebacondarwin commented Mar 6, 2017

Uh oh!

Uh oh!

devversion commented Feb 8, 2017 •

edited

Loading

devversion Feb 19, 2017 •

edited

Loading

petebacondarwin left a comment •

edited

Loading