Skip to content

feat: support emitting abstract classes for implementation types#256

Merged
mnahkies merged 7 commits into
mainfrom
mn/111/support-emit-abstract-classes
Oct 27, 2024
Merged

feat: support emitting abstract classes for implementation types#256
mnahkies merged 7 commits into
mainfrom
mn/111/support-emit-abstract-classes

Conversation

@mnahkies

Copy link
Copy Markdown
Owner

introducing a new cli flag --ts-server-implementation-method <value> that controls whether
the "implementation" contract for each router is emitted as a:

  • type (default, previous behavior)
  • interface
  • abstract class

for the abstract class case I've played around with using this with the diod DI framework over here https://github.com/mnahkies/node-scim/pull/1/files?diff=unified&w=1 and it seems to work fine, albeit its annoying having to alias the imported Implementation and createRouter symbols (this has been bugging me for a while tbh, but out of scope for this change)

@ADRFranklin

Copy link
Copy Markdown

So far this looks really good, so no complaints from me and I do remember also mentioning the problem with Implementation as the name of the interface (#111 (comment)) as I also found it to be cumbersome renaming the implementation when registering the interface with the implementation. So would love to see a fix for that at some point.

Thanks for writing up a draft so quickly, did not expect something so fast :D

@mnahkies mnahkies force-pushed the mn/111/support-emit-abstract-classes branch from 87ca2ef to c315ef0 Compare October 27, 2024 16:12
@mnahkies mnahkies marked this pull request as ready for review October 27, 2024 16:12
@mnahkies mnahkies enabled auto-merge (squash) October 27, 2024 16:13
@mnahkies mnahkies merged commit d2b8276 into main Oct 27, 2024
@mnahkies mnahkies deleted the mn/111/support-emit-abstract-classes branch October 27, 2024 16:16
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

Successfully merging this pull request may close these issues.

2 participants