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

Replace stampit with TypeScript mixins #3481

Closed
1 task done
char0n opened this issue Nov 30, 2023 · 10 comments
Closed
1 task done

Replace stampit with TypeScript mixins #3481

char0n opened this issue Nov 30, 2023 · 10 comments
Assignees
Labels
ApiDOM enhancement New feature or request

Comments

@char0n
Copy link
Member

char0n commented Nov 30, 2023

The goal of this issue is to determine if it make sense to switch from stamps to mixins. If it is determined that we're going to do that, another issue should be created to track the implementation progress there.

Pragmatically it just make sense to be able to use advanced TypeScript 5 features like decorators. Let's push through with this on OpenAPI 2.0 namespace, and see where we'll get.


TODO:

  • Documentation changes

Resources:

@char0n char0n added enhancement New feature or request ApiDOM labels Nov 30, 2023
@char0n
Copy link
Member Author

char0n commented Jan 4, 2024

Additional notes:

After further research I've come into the conclusion (subjective one), that It's currently really not worth the effort. If we would like to switch to TypeScript mixins we would have to rewrite significant amount of code and we'd still would have to use some kind of abstraction like ts-mixer to provide convenient API.

We would replace one way of doing things (IMHO more flexible with stamps) with another one, which is less flexible.

@char0n
Copy link
Member Author

char0n commented Jan 4, 2024

Thinking further...what we can do is to identify packages where using stamps is not really necessary. We really only need stamps for facilitating multiple inheritance which is not natively supported by JavaScript/TypeScript Classes - we use stamps mostly in namespace packages = packages that starts with apidom-ns-*

So for example this package: https://github.com/swagger-api/apidom/tree/main/packages/apidom-parser could be transformed to TypeScript Classes (with proper visibility).

It will create a breaking change as instead of const parser = ApiDOMParser(); we'll have to do const parser = new ApiDOMParser(); but it's still acceptable as we're in pre-release mode

We can even avoid the breaking change by utilizing the pattern for instantiating a class without a new keyword.

@char0n
Copy link
Member Author

char0n commented Jan 10, 2024

Another package that has been identified is https://github.com/swagger-api/apidom/tree/main/packages/apidom-ast

All the AST node types can be represented as TypeScript classes as there isn't multiple inheritance involved.

@glowcloud
Copy link
Contributor

The packages that I've been working on:

I'll make the suggested changes from #3646 to Parser and to the other packages, as some cases will apply there as well. The Adapter packages should be ready for a PR then. After that, I'll continue the work on the AST package.

I believe that these packages can be changed as well:

char0n added a commit that referenced this issue Jan 11, 2024
* refactor(parser-adapter-yaml-1-2): change from stamps to classes

Refs #3481

* refactor(parser-adapter-yaml-1-2): apply ordering convention

* refactor(parser-adapter-yaml-1-2): apply requested changes

* refactor: change to readonly and arrow functions

* refactor(parser-adapter-yaml-1-2): change the order of fields

---------

Co-authored-by: Vladimír Gorej <vladimir.gorej@gmail.com>
char0n added a commit that referenced this issue May 13, 2024
…ses (#4103)

Refs #3481

BREAKING CHANGE: all strategies from apidom-reference package became a class and requires to be instantiated with new operator.
char0n added a commit that referenced this issue May 13, 2024
Refs #3481

BREAKING CHANGE: all strategies from apidom-reference package became a class
char0n added a commit that referenced this issue May 13, 2024
…t classes (#4104)

Refs #3481

BREAKING CHANGE: all visitors from apidom-reference package became a class and requires to be instantiated with new operator.
char0n added a commit that referenced this issue May 14, 2024
Refs #3481

BREAKING CHANGE: IdentityManager from apidom-core package became a class and requires to be instantiated with new operator.
char0n added a commit that referenced this issue May 14, 2024
Refs #3481

BREAKING CHANGE: File from apidom-reference package became a class
and requires to be instantiated with new operator.
char0n added a commit that referenced this issue May 14, 2024
Refs #3481

BREAKING CHANGE: Reference from apidom-reference package became a class and requires to be instantiated with new operator.
char0n added a commit that referenced this issue May 14, 2024
…4100)

Refs #3481

BREAKING CHANGE: ReferenceSet from apidom-reference package became a class and requires to be instantiated with new operator.
char0n added a commit that referenced this issue May 14, 2024
…4101)

Refs #3481

BREAKING CHANGE: resolvers from apidom-reference package became a classes and requires to be instantiated with new operator.
char0n added a commit that referenced this issue May 14, 2024
)

Refs #3481

BREAKING CHANGE: parsers from apidom-reference package became a class and requires to be instantiated with new operator.
char0n added a commit that referenced this issue May 14, 2024
…ses (#4103)

Refs #3481

BREAKING CHANGE: all strategies from apidom-reference package became a class and requires to be instantiated with new operator.
char0n added a commit that referenced this issue May 14, 2024
…t classes (#4104)

Refs #3481

BREAKING CHANGE: all visitors from apidom-reference package became a class and requires to be instantiated with new operator.
@char0n
Copy link
Member Author

char0n commented May 14, 2024

Finalized in #4105

@char0n char0n closed this as completed May 14, 2024
char0n added a commit that referenced this issue May 14, 2024
Refs #3481

BREAKING CHANGE: IdentityManager from apidom-core package became a class and requires to be instantiated with new operator.
char0n added a commit that referenced this issue May 14, 2024
Refs #3481

BREAKING CHANGE: File from apidom-reference package became a class
and requires to be instantiated with new operator.
char0n added a commit that referenced this issue May 14, 2024
Refs #3481

BREAKING CHANGE: Reference from apidom-reference package became a class and requires to be instantiated with new operator.
char0n added a commit that referenced this issue May 14, 2024
…4100)

Refs #3481

BREAKING CHANGE: ReferenceSet from apidom-reference package became a class and requires to be instantiated with new operator.
char0n added a commit that referenced this issue May 14, 2024
…4101)

Refs #3481

BREAKING CHANGE: resolvers from apidom-reference package became a classes and requires to be instantiated with new operator.
char0n added a commit that referenced this issue May 14, 2024
)

Refs #3481

BREAKING CHANGE: parsers from apidom-reference package became a class and requires to be instantiated with new operator.
char0n added a commit that referenced this issue May 14, 2024
…ses (#4103)

Refs #3481

BREAKING CHANGE: all strategies from apidom-reference package became a class and requires to be instantiated with new operator.
char0n added a commit that referenced this issue May 14, 2024
…t classes (#4104)

Refs #3481

BREAKING CHANGE: all visitors from apidom-reference package became a class and requires to be instantiated with new operator.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ApiDOM enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants