stencil icon indicating copy to clipboard operation
stencil copied to clipboard

docs-type library does not resolve type/enum/interface located in another type properly with docs-json output target

Open venkatesh-nagineni opened this issue 1 year ago • 11 comments

 @stencil/[email protected]

Current behavior: I have a couple of types and enums associated with my component props, as i generating my readme files based on docs json which contain all type info with typeLibrary but some of the enums not resolving properly

Expected behavior: All types should be resolved as expected and used in component

GitHub Reproduction Link: https://codesandbox.io/p/sandbox/stencil-web-component-forked-s2vpml?file=%2Fsrc%2Fcomponents%2Fmy-component%2Fmy-component.tsx%3A24%2C1

Other information:

enum DsAlertEventStatus completely missing

and this is also missing partially in declaration type TIconType = 'error'; where it is only present "declaration": "\"error\"",

there is also TDsOptions which also produce quite different output like Array<T> which is quite unexpected

venkatesh-nagineni avatar Oct 20 '23 10:10 venkatesh-nagineni

Hey @venkatesh-nagineni 👋

Thanks for the issue report. I'm not sure I fully understand the expected behavior here, and have a few clarifying questions for you.

First, looking at the "Other Information" section of the Issue Summary:


enum DsAlertEventStatus completely missing

It looks like DsAlertEventStatus is wrapped by IDsAlertEvent like so:

export interface IDsAlertEvent {
    status: DsAlertEventStatus;
}

where IDsAlertEvent is used as such in my-component:

  @Event({ eventName: 'dsClosed' }) alertClosed$: EventEmitter<IDsAlertEvent>;

which shows up as this in the generated core.json:

"events": [
        {
          "event": "dsClosed",
          "detail": "IDsAlertEvent",
          "bubbles": true,
          "complexType": {
            "original": "IDsAlertEvent",
            "resolved": "IDsAlertEvent",
            "references": {
              "IDsAlertEvent": {
                "location": "import",
                "path": "./my-component.interface",
                "id": "src/components/my-component/my-component.interface.ts::IDsAlertEvent"
              }
            }
          },
          "cancelable": true,
          "composed": true,
          "docs": "",
          "docsTags": []
        },
...

Where "id": "src/components/my-component/my-component.interface.ts::IDsAlertEvent" resolves to the following in core.json:

    "src/components/my-component/my-component.interface.ts::IDsAlertEvent": {
      "declaration": "export interface IDsAlertEvent {\n    status: DsAlertEventStatus;\n}",
      "docstring": "",
      "path": "src/components/my-component/my-component.interface.ts"
    }

Can you help me understand what you're expecting here?


and this is also missing partially in declaration type TIconType = 'error'; where it is only present "declaration": "\"error\"",

In the my-component.interface.ts, I see this type declared as:

export type TIconType = 'error';

And gets used in my-component as:

  @Prop({ reflect: true }) iconType: TIconType;

Which looks like it's correctly resolving to me in core.json:

"props": [
        {
          "name": "iconType",
          "type": "\"error\"",
          "complexType": {
            "original": "TIconType",
            "resolved": "\"error\"",
            "references": {
              "TIconType": {
                "location": "import",
                "path": "./my-component.interface",
                "id": "src/components/my-component/my-component.interface.ts::TIconType"
              }
            }
          },
...
    "src/components/my-component/my-component.interface.ts::TIconType": {
      "declaration": "\"error\"",
      "docstring": "",
      "path": "src/components/my-component/my-component.interface.ts"
    },

Can you help me understand what you're expecting here and why?


there is also TDsOptions which also produce quite different output like Array<T> which is quite unexpected

Can you help me understand what you're expecting here and why?


Finally, from the "Expected Behavior" section:

All types should be resolved as expected and used in component

Are there any other types that haven't been mentioned here in my comment or in the issue summary that you expect?

rwaskiewicz avatar Oct 20 '23 12:10 rwaskiewicz

Thanks for the quick response!

When it comes to documenting interfaces, it's essential to provide comprehensive information, including second-level associated types.

Let's consider Case 1:

export interface IDsAlertEvent {
    status: DsAlertEventStatus;
}

In this scenario, we've properly resolved the first-level interface IDsAlertEvent. However, consumers might also need to understand what DsAlertEventStatus is. So, what I'm expecting is to receive complete information associated with the component:

export enum DsAlertEventStatus {
    Closed = 'closed',
    Autohide = 'autoHide',
}

export interface IDsAlertEvent {
    status: DsAlertEventStatus;
}

By having this complete information, I can use it when generating my readme files, instead of having to generate or resolve it on my own without the use of a type library.

Now, let's move to Case 2:

We are exporting enum/interface names, but the same isn't happening for types. For types, I'm also expecting a similar treatment, like so:

export type TIconType = 'error'; Screenshot 2023-10-21 at 10 09 29 Screenshot 2023-10-21 at 10 12 23

Finally, in Case 3:

I expect the following to be available because a prop is using a type that combines the other two interfaces I need to include in my document generation:

export interface IOption {
    label: string;
    value: string;
    disabled?: boolean;
    selected?: boolean;
}

export interface IOptionGroup {
    label: string;
    options: IOption[];
    disabled?: boolean;
}

export type TDsOptions = (IOption | IOptionGroup)[];

This ensures that I have all the necessary information for my document generation process, especially when types are involved in the interfaces."

venkatesh-nagineni avatar Oct 21 '23 08:10 venkatesh-nagineni

@rwaskiewicz have you considered case 3 https://codesandbox.io/p/devbox/stencil-web-component-forked-s2vpml?file=%2Fcore.json

venkatesh-nagineni avatar Mar 08 '24 09:03 venkatesh-nagineni

Hey @venkatesh-nagineni, just want to clarify a few things!

For the issue with TIconType is your expectation that you'll have something like

    "src/components/my-component/my-component.interface.ts::TIconType": {
      "declaration": "type TIconType = \"error\"",
      "docstring": "",
      "path": "src/components/my-component/my-component.interface.ts"
    },

just wondering because as @rwaskiewicz pointed out the current output is this:

    "src/components/my-component/my-component.interface.ts::TIconType": {
      "declaration": "\"error\"",
      "docstring": "",
      "path": "src/components/my-component/my-component.interface.ts"
    },

this is because "error" is a complete description of what the TIconType is, i.e. we can't really give any more info than that, it's declared (in source) as

export type TIconType = 'error';

the type TIconType is a name (or a 'type alias') for the concrete type "error", so there isn't really more we could possibly include in the type library for that one.

It looks like the other issues you're facing here are related to declarations that you'd expect to be included in the type library not being included. If that's an accurate assessment, then I think there's a feature built in to the docs-json output target that could help!

If you set the supplementalPublicTypes option on the output target in your stencil.config.ts you can indicate an additional file where every type that you export from it will be automatically added to the type library. A good way to use this would be to have a barrel file in src, like src/public-types.ts or something, and any type which isn't being pulled in already could be exported from there. Then those types should show up in the type library!

More on that here: https://stenciljs.com/docs/docs-json#supplementalpublictypes

I forked your codesandbox and added that option, setting it to ./src/components/my-component/my-component.interface.ts and now things like the IOption and DsAlertEventStatus are now included, like this:

    "src/components/my-component/my-component.interface.ts::IDsAlertEvent": {
      "declaration": "export interface IDsAlertEvent {\n    status: DsAlertEventStatus;\n}",
      "docstring": "",
      "path": "src/components/my-component/my-component.interface.ts"
    },
    "src/components/my-component/my-component.interface.ts::DsAlertEventStatus": {
      "declaration": "export enum DsAlertEventStatus {\n    Closed = 'closed',\n    Autohide = 'autoHide',\n}",
      "docstring": "",
      "path": "src/components/my-component/my-component.interface.ts"
    },
    "src/components/my-component/my-component.interface.ts::IOption": {
      "declaration": "export interface IOption {\n    label: string;\n    value: string;\n    disabled?: boolean;\n    selected?: boolean;\n}",
      "docstring": "",
      "path": "src/components/my-component/my-component.interface.ts"
    },
    "src/components/my-component/my-component.interface.ts::IOptionGroup": {
      "declaration": "export interface IOptionGroup {\n    label: string;\n    options: IOption[];\n    disabled?: boolean;\n}",
      "docstring": "",
      "path": "src/components/my-component/my-component.interface.ts"
    }

you can check that out here

alicewriteswrongs avatar Apr 10 '24 19:04 alicewriteswrongs

@alicewriteswrongs thx for the reply

seems we are getting different output json generated even use same version, i also tested what you share the test link which is working your side but not the same for me (tested with latest 4.17.0 version)

i don't see this in my build

    "src/components/my-component/my-component.interface.ts::DsAlertEventStatus": {
      "declaration": "export enum DsAlertEventStatus {\n    Closed = 'closed',\n    Autohide = 'autoHide',\n}",
      "docstring": "",
      "path": "src/components/my-component/my-component.interface.ts"
    },

if we have type with more than one union type then it returns properly but with only one value then it just return the value but not the type name

export type TIconType = 'error' | 'warning'

this giving 

 "declaration": "export type TIconType = \"error\" | \"warning\";",

where as like this

export type TIconType = 'error';

then it is giving

\"error\"  but expecting export type TIconType = \"error\"

regarding different issue

union array type this is working

export type TDsOptions = IOption[] | IOptionGroup[];

but not this one, produce wierd output in typelib

export type TDsOptions = (IOption[] | IOptionGroup)[];

Screenshot 2024-04-23 at 15 29 57

regarding third issue, i don't see DsAlertEventStatus like you get (strange)

Screenshot 2024-04-23 at 15 33 29

this is the typeLibrary node i get https://codesandbox.io/p/devbox/stencil-web-component-forked-w8lzn2?file=%2Fcore.json

venkatesh-nagineni avatar Apr 23 '24 13:04 venkatesh-nagineni

Hey @venkatesh-nagineni, thanks for providing more info, I'm not sure why you're not seeing the same output on codesandbox, perhaps try building locally instead where you can clear out all the caches and whatnot. Most of the browser-based dev environments (like codesandbox, stackblitz, etc) do some caching behind the scenes to make the experience performant, which has caused problems with Stencil in the past.

On the issue with your type like

export type TIconType = 'error';

just to clarify, your expectation is that you get something like

    "src/components/my-component/my-component.interface.ts::TIconType": {
      "declaration": "export type TIconType = \"error\"",
      "docstring": "",
      "path": "src/components/my-component/my-component.interface.ts"
    },

rather than

    "src/components/my-component/my-component.interface.ts::TIconType": {
      "declaration": "\"error\"",
      "docstring": "",
      "path": "src/components/my-component/my-component.interface.ts"
    },

to the string we put under the declaration field we use methods provided by the typescript type checker. For a type like 'foo' | 'bar' it will return something like type Foo = 'foo' | 'bar' but in my testing when a type is an alias just for a single string value (like type Foo = 'foo') the string 'foo' is what we get.

Perhaps it would help to understand why getting "error" for the declaration for TIconType is a problem for your use-case?

alicewriteswrongs avatar Apr 23 '24 14:04 alicewriteswrongs

thx @alicewriteswrongs for the quick reply

we also display type info for my consumers , showing only"error" which is not cool and also it is not giving deep references (AST tree is not enough, as i have some complex types but our typeLibrary does not give all deep complex references associated with the prop or event)

currently we solve this issue to write on our own to generate type info with Typescript Api typechecker but would be nice to get deep tree in our typeLibrary lib

regarding DsAlertEventStatus missing, this is actually found during my prod then i created codesandbox environment with the example but i will check again

venkatesh-nagineni avatar Apr 23 '24 15:04 venkatesh-nagineni

Can you explain what you mean by 'deep tree' for the types in the library, possibly with an example?

alicewriteswrongs avatar Apr 23 '24 17:04 alicewriteswrongs

@alicewriteswrongs here it is reproducible example

basically i have a prop with complex interface, our typeLibrary resolves topp level node where we need to go deep and grab all references associated with that top node

https://codesandbox.io/p/devbox/stencil-web-component-forked-vklhzw?file=%2Fcore.json

expected to have NestedObjectType, SomethingElse etc... so that i can able to display this info to my consumers

Screenshot 2024-04-24 at 07 51 22

venkatesh-nagineni avatar Apr 24 '24 05:04 venkatesh-nagineni

Gotcha, I understand what you're trying to do and the way in which this isn't quite working as-expected right now.

However, this behavior is due to a limitation that we intentionally added during the implementation of the type library feature. It may not be obvious at first why this is the case, but we can't simply pull all referenced types in a type declaration into the type library without opening up some potential issues. In particular, we'd have to deal with types that are external types but may be referenced by an interface.

For instance, consider if I had something like:

import type { Plugin } from 'rollup';

export interface MyGreatInterface {
  name: string;
  numbers: number[];
  plugins: Plugin[];
}

If the MyGreatInterface type is added to the type library and we recursively add all referenced types in the type library then we'd have to pull in the Plugin type from Rollup, which then itself could necessitate pulling in a whole bunch of other types.

So to avoid that issue we decided not to add recursive inclusion of other types. The supplementalPublicTypes configuration option that I mentioned above is the way to address this situation, you can add exports to your supplemental file for all of the additional types that you want to have exported.

alicewriteswrongs avatar Apr 24 '24 13:04 alicewriteswrongs

Ok, partially agreed

in real world scenario we do not have such complex props to the consumer (consumer can not pass Plugin), currently docs-json pulls props, events and methods type info to the typeLibrary

I wonder how ionic will handle this if you have at least one reference associated with main interface but not like how i provided is just an example, in my real use case i do not such real complex interface

adding supplementalPublicTypes bit hard where we have interfaces in each component, off course currently i am collecting all interfaces through out src folder with typescript api

anyway i did workaround and already worked like a charm, thx for the clarification

venkatesh-nagineni avatar Apr 25 '24 06:04 venkatesh-nagineni