typedoc icon indicating copy to clipboard operation
typedoc copied to clipboard

Support computed property names

Open protyposis opened this issue 6 years ago • 9 comments

Problem

Typedoc does not support computed property names. For example the following code

export const propertyName1 = 'p1';
export const propertyName2 = 'p2';

export const Object = {
  [propertyName1]: 'foo',
  [propertyName2]: 'bar',
};

Results in this output:

image

Suggested Solution

Expected output would be something along this (with the computed properties additionally being linked to the variables in some way):

image

I've been needing this multiple times in the past years and always went with workarounds (none of which was nice and clean). I am wondering why there is not a single issue existing about computed properties. Am I missing something?

protyposis avatar Jan 09 '19 11:01 protyposis

Maybe we could start from here? At this stage you get the name __computed.

src/lib/converter/factories/declaration.ts

// Ensure we have a name for the reflection
    if (!name) {
        if (node.localSymbol) {
            name = node.localSymbol.name;
        } else if (node.symbol) {
            name = node.symbol.name;
        } else {
            return;
        }
    }

9oelM avatar Jan 09 '19 14:01 9oelM

Checking kind of child inside node will return 149(ComputedPropertyName). That's one way we could check if the node contains a ComputedPropertyName:

const valueDeclaration = node.symbol.valueDeclaration
// Object properties are always at the first index of `children`
const firstChild = valueDeclaration.getChildAt(0, valueDeclaration.getSourceFile()) 

if(firstChild.kind === ts.SyntaxKind.ComputedPropertyName){
  // do something with it
  // ...
  firstChild.getText(firstChild.getSourceFile()) //  [propertyName1] like in the example
}

9oelM avatar Jan 09 '19 17:01 9oelM

So a temporary fix would be a few lines of additions of codes in src/lib/converter/factories/declaration.ts:

// Ensure we have a name for the reflection
    if (!name) {
        if (node.localSymbol) {
            name = node.localSymbol.name;
        } else if (node.symbol) {
            const valueDeclaration = node.symbol.valueDeclaration
            const firstChild = valueDeclaration.getChildAt(0, valueDeclaration.getSourceFile())
            if(firstChild.kind === ts.SyntaxKind.ComputedPropertyName){
                // insert raw name for property name
                name = firstChild.getText(firstChild.getSourceFile()) 
            }
            else{
                name = node.symbol.name;
            }
        } else {
            return;
        }
    }

Then, this is going to render: image

Oh. by the way, test.ts is pretty much the same as your source.

const prop1 = 'p1';
const prop2 = 'p2';

const RenderTest = {
  [prop1]: '1',
  [prop2]: '2',
};

But I'm sure there's a much better way to do it and I know it's much more complex. This is more of tweaking. And this will not add a link to the original variable, so some more work is needed.

9oelM avatar Jan 09 '19 17:01 9oelM

@9oelM Nice research! I'm wondering if we can find something more reliable than just checking the first child to see if it is a computed property name.

aciccarello avatar Jan 10 '19 02:01 aciccarello

Just gave @9oelM's code a try and it works quite nicely :) Had to add an additional null check for firstChild to avoid crashing in some other cases:

    if (!name) {
        if (node.localSymbol) {
            name = node.localSymbol.name;
        } else if (node.symbol) {
            const valueDeclaration = node.symbol.valueDeclaration;
            const firstChild = valueDeclaration ? valueDeclaration.getChildAt(0, valueDeclaration.getSourceFile()) : null;
            if(firstChild && firstChild.kind === ts.SyntaxKind.ComputedPropertyName){
                // insert raw name for property name
                name = firstChild.getText(firstChild.getSourceFile());
            }
            else{
                name = node.symbol.name;
            }
        } else {
            return;
        }
    }

It also works nicely for computed properties from enum s which was my actual use case :)

Now the only thing missing is to link the computed properties as mocked here:

image

Does anyone have an idea how to achieve that? Are links in property names even supported yet?

protyposis avatar Jan 16 '19 08:01 protyposis

I assume the url of the DeclarationReflection in src/lib/converter/factories/declaration.ts needs to be set to the declaration used in the computed property. The URL can then be rendered as a link instead of text by updating member.declaration.hbs. What I could not figure out is where the url actually needs to be set (setting a dummy value in declaration.ts works as described).

protyposis avatar Jan 16 '19 09:01 protyposis

@9oelM Thanks for your solution. In my case 0 was not the ComputedProperty because I have comments. So I improved a bit your solution to check where is the ComputedProperty in the childrens.

if (!name) {
        if (node.localSymbol) {
            name = node.localSymbol.name;
        } else if (node.symbol) {
            const valueDeclaration = node.symbol.valueDeclaration;
            const computedChild = valueDeclaration &&
                valueDeclaration.getChildren().find(child => child.kind === ts.SyntaxKind.ComputedPropertyName);
            if (computedChild) {
                name = computedChild.getText(computedChild.getSourceFile());
            } else {
                name = node.symbol.name;
            }
        } else {
            return;
        }
    }

Hope someone finds the way to make the links to the properties.

ivangonzalezsp avatar Jan 22 '19 11:01 ivangonzalezsp

I will have a closer look later as I am not really available as of now. Sorry about that :(

9oelM avatar Jan 24 '19 03:01 9oelM

#1039 has improved our support for computed property names, but not fully solved the issue.

To properly handle this, we would need to add a new property to declaration reflections that describes a computed name, and change the current name property to be a getter that either returns a plain string name or constructs the name from the computed name.

Gerrit0 avatar Dec 08 '19 19:12 Gerrit0