Skip to content

Added variants handling #234

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

Merged
merged 14 commits into from
Mar 29, 2021
Merged

Added variants handling #234

merged 14 commits into from
Mar 29, 2021

Conversation

delvedor
Copy link
Member

@delvedor delvedor commented Mar 26, 2021

As titled.

External variant example:

/** @variants external */
type FooContainer = Faz | Bar

/** @variant name='faz' */
class Faz {
  faz: string
}

/** @variant name='bar' */
class Bar {
  bar: string
}

Internal variant example:

class TestPolymorphicBase {
  baz: boolean
}

class TestPolymorphic extends TestPolymorphicBase {
  type: 'heya'
}

enum Foo {
  hello = 1,
  ciao = 2
}

class TestPolymorphic2 extends TestPolymorphicBase {
  type: Foo
}

/** @variants internal tag='type' */
type TestPolymorphicType = TestPolymorphic | TestPolymorphic2

TODO:

Mpdreamz
Mpdreamz previously approved these changes Mar 26, 2021
Copy link
Member

@swallez swallez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loving how we end up with something simple yet capturing all that is needed for strongly typed languages!

Added some comments to add a 3rd variant kind container.

@@ -179,6 +200,8 @@ export class Interface extends BaseType {
*/
attachedBehaviors?: string[]
properties: Property[]
/** Identify containers */
variants?: ExternalTag
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be:

Suggested change
variants?: ExternalTag
variants?: Container

swallez
swallez previously approved these changes Mar 26, 2021
Copy link
Member

@swallez swallez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@delvedor delvedor marked this pull request as ready for review March 29, 2021 06:35
@delvedor
Copy link
Member Author

This is good to go 🎉
I've fixed all *Property issues, there are still a few errors in indices.put_mapping, but not related to this.

@delvedor delvedor requested review from swallez and Mpdreamz March 29, 2021 06:40
swallez
swallez previously approved these changes Mar 29, 2021
Copy link
Member

@swallez swallez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (still found a typo)

Awesome stuff, I'll add the @variants annotations and additional model validations in a follow-up PR.

@delvedor delvedor merged commit 657f383 into master Mar 29, 2021
@delvedor delvedor deleted the polymorphic branch March 29, 2021 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants