search

ionic-team / stencil-ds-output-targets

These are output targets that can be added to Stencil for React and Angular.
https://stenciljs.com
MIT License
247 stars 111 forks source link

feat: Generate type imports from relative path in Angular output #258

Open FabioGimmillaro opened 2 years ago

FabioGimmillaro commented 2 years ago

Prerequisites

Stencil Version

0.15.1

Stencil Framework Output Target

Angular

Stencil Framework Output Target Version

0.2.0 - 0.4.0

Current Behavior

The proxies file generated by the angular-output-target step contains false imports for interfaces and types defined within custom typescript files.

e.g. import type { DateError as IComponentDateError } from '@module/project';

Expected Behavior

All types and interfaces not defined within a stencil component should set the correct imports within the proxy file.

e.g. import type { DateError as IComponentDateError } from '@module/project/dist/types/path';

Steps to Reproduce

  1. Setup projects described in https://stenciljs.com/docs/angular
  2. generate file in stencil project with exported custom interface/type (e.g: export type Test = "test";)
  3. add Event to stencil component with interface/type output (e.g. @Event() test: EventEmitter;)
  4. run stencil build
  5. Got to proxy output (path should be ${angular-project-path}/projects/component-lib/src/lib/stencil-generated/components.ts if not defined otherwise)
  6. import of type Test => import type { Test as IMyComponentTest } from '@test/test';
  7. => Import should be import type { Test as IMyComponentTest } from '@test/test/dist/types/components/classes/test'; ';

grafik grafik

or

  1. Checkout https://github.com/FabioGimmillaro/stencil-proxy-bug
  2. npm install in both projects (stencil and angular)
  3. Got to test-angular/projects/component-library/src/lib/stencil-generated/proxies.ts
  4. In Line 10 you will find the broken import.

Code Reproduction URL

https://github.com/FabioGimmillaro/stencil-proxy-bug

Additional Information

The angular-output-target version 0.1.0 set the correct import paths for all types and interfaces used within the EventEmitter.

There was already a bug submitted which described this issue: https://github.com/ionic-team/stencil-ds-output-targets/issues/197

I don't think the solution to add your interfaces or types to an interface.d.ts is very efficient. Maybe there is a better solution which I couldn't find?

sean-perkins commented 2 years ago

Currently, the implementation expects you to export all your library types from a top-level entry file (i.e. interface.d.ts). Can you share why this approach is not efficient?

Likely we could maintain the file path of the type when generating these files, but that requires a lot more complexity and test coverage to support; which delays when this change could be introduced into the project.

FabioGimmillaro commented 2 years ago

Currently, the implementation expects you to export all your library types from a top-level entry file (i.e. interface.d.ts). Can you share why this approach is not efficient?

Likely we could maintain the file path of the type when generating these files, but that requires a lot more complexity and test coverage to support; which delays when this change could be introduced into the project.

Hi Sean,

thanks for your reply. First you either have to adjust the interface.d.ts everytime a new type/interface/class is defined in a new file. Because we mantain our types in relative paths to their context it is no option to declare every new type/interface in a single file. So our only option is to either adjust the interface.d.ts everytime a new type is defined or we write a custom generator which automatically checks for types used in EventEmitters and write them to the interfaces.d.ts automatically.

sean-perkins commented 2 years ago

Thanks for the follow-up! I'll mark this as a feature for now, as it is going to require both some investigation into the feasibility to maintaining the relative path structure to the types as well as a refactor to how those type imports get injected into the build output.

Likely each output target in this repo behaves much in a similar way, so it would be great to address all of them in tandem.

sbahramy-meltwater commented 1 year ago

Hi @sean-perkins, we are having the same issue that @FabioGimmillaro raised here. This doesn't seem to "work out of the box". If you follow the docs and simply add a custom type, the angular output then generates a broken proxy file. Following your suggestion about the top-level interface.d.ts (docs have it plural interfaces.d.ts) doesn't seem to fix it either. Am I missing something? Thank you.

sean-perkins commented 1 year ago

@sbahramy-meltwater without a reproduction it is hard to know for certain.

The name of the file ultimately does not matter. In Ionic Framework we call it interface.d.ts, but it can be any name of your choosing (as long as it is a type declaration file *.d.ts).

You will need to update the package.json of your library to reference whatever name you choose:

{
  ...
  "types": "dist/types/interface.d.ts",
}
sbahramy-meltwater commented 1 year ago

@sean-perkins I forked Fabio's repo he supplied when opening this ticket and made a couple of adjustments. If you clone this repo down and follow the below steps, you can see the generated components.d.ts file has a broken import path for the type. This project is set up like in the docs (folder names are different though) and is kept very simple - one component, and one custom type.

  1. cd into test folder and npm i
  2. run npm run build
  3. cd into test-angular/projects/component-library and npm i
  4. view the generated components.ts file and notice the broken path
  5. Add in the top level interfaces.d.ts in test/src with only this line in it: export * from './components';
  6. Update package.json to have "types": "dist/types/interfaces.d.ts"
  7. cd back into test and run npm run build
  8. The components.ts is re-generated but has the same issue.

Please let me know how else I can help on this.