stencil icon indicating copy to clipboard operation
stencil copied to clipboard

feat: allow imports of reusable Enum types

Open MatteoFeltrin opened this issue 1 year ago • 9 comments

Prerequisites

Stencil Version

4.22

Current Behavior

Inside a component we have this code:

// ...
// omitted useless code
// ...

export interface ConfirmationService {
    confirm(): void;
}

@Component({
    tag: 'my-component',
    shadow: true,
})
export class MyComponent{
// ...
// omitted useless code
// ...

Everything is compiling and working correctly. This works only with interface, exporting Classes or Functions throws an error. Is this fine and we can do that, or it may lead to unexpected behaviour and problems with the bundling as documented?

Expected Behavior

As documented here https://stenciljs.com/docs/module-bundling#one-component-per-module I would expect the compiler to throw something like:

[ ERROR ]  src/components/my-component.tsx:4:1
        To allow efficient bundling, modules using @Component() can only have a single export which is the component
        class itself. Any other exports should be moved to a separate file. For further information check out:
        https://stenciljs.com/docs/module-bundling

  L4:  export interface ConfirmationService()

System Info

No response

Steps to Reproduce

Export an interface from a component entry point.

Code Reproduction URL

none

Additional Information

No response

MatteoFeltrin avatar Oct 16 '24 07:10 MatteoFeltrin

Is this fine and we can do that, or it may lead to unexpected behaviour and problems with the bundling as documented?

Hey @MatteoFeltrin 👋 thanks for raising the issue. Exporting interfaces is totally fine as these are being scrapped from the bundled files and only will be seen in the .d.ts files.

Let me know if you have any further questions.

christian-bromann avatar Oct 16 '24 13:10 christian-bromann

@christian-bromann Thanks for clarifying!

I was thinking to open a pull request to update the documentation. Do you think it is worth it?

Another doubt I have is: Is not possibile to assign a type to a @Prop which is imported from a non-component file?

import MyEnum from "../../../common/MyEnum ";
// ....
@Component({tag: 'my-component'})
export class MyComponent{
    @Prop() inputType!: MyEnum;
// ....

MyEnum .ts

enum MyEnum {
  First= '1',
  Second = '2',
}

export default MyEnum ;

The import from MyEnum is missing inside components.d.ts after compiling and the compiler is throwing this error:

  TypeScript: src/components.d.ts:18:22
           Cannot find name 'MyEnum'.

I found this issue that is close to this problem: https://github.com/ionic-team/stencil/issues/3124

rwaskiewicz has updated the documentation here https://github.com/ionic-team/stencil-site/pull/784 but is not clearly specified whether the "exported type" MUST be inside a component file or not.

Anyway Enums are those which cannot be exported inside a Component file, so it would not be possible to use it.

MatteoFeltrin avatar Oct 16 '24 13:10 MatteoFeltrin

Yeah .. Enums are not just a type, they represent an actual object in JavaScript which causes this limitation. I am honestly not too familiar with this optimization in Stencil and have experienced the same in the past. At this point, if you think we can improve the docs for this, I would appreciate any contribution to our docs page.

christian-bromann avatar Oct 16 '24 14:10 christian-bromann

Ok, I will do that!

Do you have any advice on how I can assign to @Prop a type that needs to be imported from "common" files that are not components?

My use case: I have many "base" components that uses md-outlined-text-field, a component from MaterialDesign/web, and I would like to use a "common" Enum with all the possible "type" values of this TextField

// (from MaterialDesign documentation)
type="text"
type="email"
type="number"
type="password"
type="search"
type="tel"
type="url"
type="textarea"

MatteoFeltrin avatar Oct 16 '24 14:10 MatteoFeltrin

This sounds like a valid issue that should work or if not should be supported. I've renamed the issue and will re-open.

christian-bromann avatar Oct 16 '24 15:10 christian-bromann

@christian-bromann 🚀

Let me know if there is anything I can do to help with the issue

MatteoFeltrin avatar Oct 16 '24 15:10 MatteoFeltrin

Let me know if there is anything I can do to help with the issue

Feel free to dive into it. I can't say when/if the Stencil team has time to take a look.

christian-bromann avatar Oct 16 '24 15:10 christian-bromann

@christian-bromann I have dived into it and found out that only NamedImports are considered by the components.d.ts generator. If you use a DefaultImport it is considered as a global variable.

I explain it better with an example: MyEnum.ts

enum MyEnum {
  Case1= '1',
  Case2= '2',
}
export default MyEnum;

component

import MyEnum from "../MyEnum ";
// ....
@Component({tag: 'my-component'})
export class MyComponent{
    @Prop() inputType!: MyEnum;
// ....

Inside the generated components.d.ts you will find this: image The import statement of MyEnum is missing beucase getTypeReferenceLocation considers it as a Global.

If on the other hand you use Named exports like this MyEnum.ts

export enum MyEnum {
  Case1= '1',
  Case2= '2',
}

component

import { MyEnum } from "../MyEnum ";
// ....
@Component({tag: 'my-component'})
export class MyComponent{
    @Prop() inputType!: MyEnum;
// ....

Everything works fine and inside components.d.ts you will have: image

I was trying to fix it but it is not an easy one, and what gives me many doubts is that it seems to be developed only for NamedImports on purpose. May I ask you if Stencil team can confirm me that this fix really need to be done, or if there is any reason why they didn't support Default exports? Just to avoid making a hard work for nothing!

Thanks!

MatteoFeltrin avatar Oct 17 '24 14:10 MatteoFeltrin

May I ask you if Stencil team can confirm me that this fix really need to be done, or if there is any reason why they didn't support Default exports?

Honestly this is impossible to answer given that no one who has started Stencil is around anymore. My guess is that the original authors of the framework wanted to ensure that components can only my exported via named exports to ensure they have a unique id. They may have missed the case where someone would like to export other primitives like enums etc. Furthermore there is also the issue with naming collisions and how to avoid that.

That said, I feel confident that we would want to allow default exports. Restricting it seems to create a lot of confusion. Alternatively we could also have the compiler print a warning in case default exports are detected.

christian-bromann avatar Oct 29 '24 21:10 christian-bromann