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

Clearly define how traits should be applied #36

Closed
jonaslagoni opened this issue Dec 17, 2021 · 3 comments
Closed

Clearly define how traits should be applied #36

jonaslagoni opened this issue Dec 17, 2021 · 3 comments
Labels
enhancement New feature or request stale

Comments

@jonaslagoni
Copy link
Member

Reason/Context

Because traits can be somewhat hard to get right, we need to ensure that all parsers apply them correctly and provide all the necessary documentation to achieve it.

Related issues:
Spec issue: asyncapi/spec#505
Spec PR: asyncapi/spec#532

@Fannon
Copy link

Fannon commented Dec 17, 2021

Thanks for bringing this up!

In this JSFiddle I've provided two example implementations in JavaScript, in the case that we want to go with the approach proposed in #532:

The first implementation (function apply(origin, patch)) is based on JSON Merge patch and deletes target properties if null is passed.

The second implementation (function merge(...objects)) is very similar, but skips on the special null behavior (my personal preference)

In general, I'm unsure whether we should base trait inheritance on the JSON Merge patch RFC, as its semantics are different from what we want to have from traits. Merge Path is basically about the patch overwriting the target. Inheritance / Traits are about applying information from the source to the target, but never overwriting the target.
I'm also unsure on the Merge Patch behavior of using null to remove a property. This is essential for patches, but I can see how this is misleading (and I think even unnecessary) for trait inheritance.

Please be aware that there are no unit-tests for this implementation. Before putting it into the spec, we should write unit-tests with various example data.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Apr 17, 2022
@github-actions github-actions bot removed the stale label Jul 29, 2022
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Nov 27, 2022
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request stale
Projects
None yet
Development

No branches or pull requests

2 participants