runem / web-component-analyzer

CLI that analyzes web components and emits documentation
https://runem.github.io/web-component-analyzer
MIT License
507 stars 65 forks source link

Support type aliases in class declaration JSDoc tags #152

Open calebdwilliams opened 4 years ago

calebdwilliams commented 4 years ago

I might have missed this in the docs somewhere, but I would love the ability to define my types using TypeScript but write my source in good ol' ES files with JSDoc. TypeScript already provides type checking for this set up. I'd love it if I could tell wca to look at TS files for complex type definitions and interfaces to keep my JS relatively clean. Is there a way to accomplish this?

runem commented 4 years ago

Hi Caleb, thanks for opening this issue :-)

I use tsc directly, so it should pretty much provide the same capabilities as if you ran tsc on your files. Do you have a specific code example so I can see what you are trying to achieve?

calebdwilliams commented 4 years ago

Unfortunately my example is on my company’s GitHub. What’s funny is when I was running the command without an outFile, I was seeing the right text. Adding outFile seemed th change that. I’ll try to put together a reproduction.

calebdwilliams commented 4 years ago

Hey @runem, here's the reproduction for this issue.

runem commented 4 years ago

Thank you so much for the repro :-)

First of all, I think the JSDoc notation is invalid: @attr greeting {Greeting} - Who should we greet? should be @attr {Greeting} greeting - Who should we greet?

That should be the reason for why it doesn't find the attribute.

In addition, you could remove the @attr JSDoc tag from the class declaration and instead add this to the property like this:

constructor() {
    super();

    /** 
      * Who should we greet?
      * @attr
      * @type {Greeting} 
      */
    this.greeting = '';
}

And just to be clear, do you expect the output to be this?

Property Attribute Type Default
greeting greeting 'world' \| 'y\'all' ""

or would this be what you want?

Property Attribute Type Default
greeting greeting Greeting ""
calebdwilliams commented 4 years ago

I would expect option one above where type is set as 'world | 'y\'all', but maybe that's wrong-headed. If it does come out as Greeting, I do feel like there should be a section of the docs for non-standard types so consumers of the element will know what the valid options are.

To be clear, changing the code to what you've added above still has type set as Greeting. Is this the the intended behavior? Like I said, I might be missing something or making a poor assumption (Lord knows it wouldn't be the first time).

runem commented 4 years ago

Yes, this is actually the intended behavior :-)

However, here you can see a feature request requesting a way to inline types: #140, which sounds like what you want. This functionality will be implemented behind the flag --inline-types.

I like the point you made about having a section of the markdown docs for non-standard types. I just yet haven't found out how this would best be implemented.

Finally, I think if you just use the JSDoc tag like this: @attr {Greeting} greeting - Who should we greet? on the class declaration, it wouldn't find the correct type to inline and it would just end up being the non-inlined Greeting type anyway (because it doesn't resolve type aliases from JSDoc using @attr). You should however be fine by adding @attr to the corresponding property instead as mentioned. Therefore, I would like to rename this issue to "Support type aliases in class declaration JSDoc tags" in order to specifically focus on that bug, if that's fine for you.

calebdwilliams commented 4 years ago

Yeah, that's great. I'm more than happy having it actually say Greeting, so long as Greeting is defined, in the docs, FWIW.

Side question, how does custom-elements.json handle custom types?

runem commented 4 years ago

That's a good question. In the existing experimental JSON format, the type is just a type hint which is non-parsable string that describes the type. However, we want to move to a place where custom types are actually parsable when reading the JSON. This turns out to be a bit difficult and could be implemented with anything from JSDoc type expressions to Typescript declaration files. It's still very much up for discussion.

You can also read this comment which I just wrote regarding adding methods to the JSON. I think gives a good summary :-)

https://github.com/runem/web-component-analyzer/issues/146#issuecomment-597280616

There exists two PR's with concrete suggestions on how the format could look :-) The problem right now with the experimental JSON format that web-component-analyzer outputs, is that it becomes really difficult to output more complex structures such as custom types, modules, inheritance, methods, eg.

This PR opens up for a JSON structure that allows outputting more complex concepts. I intend to implement that structure as parallel experimental JSON format that will temporarily exist side by side with the existing experimental JSON format. This way we can experiment and play around with concrete outputs of the JSON format in order to better access the optimal output. This output format will include methods.

All in all, I think that there is consensus that methods should be included in the final JSON format. The question is however, how to model it in JSON.

calebdwilliams commented 4 years ago

I realize this is outside the scope of this particular issue, but with custom-elements.json, I would imagine there would be a types field outside of tags. This way the type could be referenced by potentially multiple tags. The question then, of course, is how you model the relationship and how you model the custom type. JSON Schema, has syntax that's been tested fairly well, I believe …