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

Support type aliases in class declaration JSDoc tags #152

Open
calebdwilliams opened this issue Mar 6, 2020 · 9 comments
Open

Support type aliases in class declaration JSDoc tags #152

calebdwilliams opened this issue Mar 6, 2020 · 9 comments

Comments

@calebdwilliams
Copy link

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
Copy link
Owner

runem commented Mar 6, 2020

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
Copy link
Author

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
Copy link
Author

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

@runem
Copy link
Owner

runem commented Mar 10, 2020

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
Copy link
Author

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
Copy link
Owner

runem commented Mar 10, 2020

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
Copy link
Author

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
Copy link
Owner

runem commented Mar 10, 2020

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 :-)

#146 (comment)

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
Copy link
Author

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 …

@calebdwilliams calebdwilliams changed the title Have wca read TS files in a primarily JS repo Support type aliases in class declaration JSDoc tags Mar 10, 2020
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

No branches or pull requests

2 participants