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

Metadata decorator Inheritability behaviour? #343

Closed
ktutnik opened this issue Oct 18, 2020 · 4 comments
Closed

Metadata decorator Inheritability behaviour? #343

ktutnik opened this issue Oct 18, 2020 · 4 comments

Comments

@ktutnik
Copy link

ktutnik commented Oct 18, 2020

Congrats for the new decorator proposal, look like it causing less breaking change than the previous one.

Anyway... Regarding below description from the current Readme.

Decorators may also annotate a class element with metadata. These are simple, unrestricted object properties, which are collected from all decorators which add them, and made available as a set of nested objects in the [Symbol.metadata] property.

I concern about its Inheritability behaviour. In an ORM framework it's a common use case to have an Entity inherited from a super class.

class EntityBase {
  @primaryGeneratedColumn()
  id

  @column({ default: false })
  deleted
}
class Post extends EntityBase{
    @column()
    title
}

Giving code above, will the Post[Symbol.metadata].instance.fields.deleted work?

Will we have more control to this behaviour such as enable/disable the inheritance behaviour? or even control the multiple behaviour like in .Net framework.

As a reference, In .Net framework, inheritability behaviour of metadata attribute is like below:

  1. There are two options Inheritable and AllowMultiple.
  2. If it's inheritable and if there are the same attributes applied on derived class than if its AllowMultiple then multiple metadata applied, if not, only metadata from the derived class applied.
  3. Allow multiple also used to control if the same decorator can be applied without inheritance context. This behavior supported language static check.
@littledan
Copy link
Member

I think this is all about how you query the metadata. As @rbuckton has pointed out, we will likely need a standard library to query it in a way that respects inheritance, like Reflect.metadata does. The README also has an example of querying the metadata in a way that respects inheritance (in the @on decorator).

@ktutnik
Copy link
Author

ktutnik commented Oct 20, 2020

In context of inheritance, its correct! it's all about the query. But in context of AllowMultiple I think its will be a bit different.

In the current decorator proposal, metadata decorator is "Creating the metadata", since in .Net framework "The decorator is the metadata itself". It's important because when the decorator is the metadata itself its easy to identify that a decorator is applied multiple time (easy to identify the uniqueness), so system can provide check automatically. The current decorator proposal will be possible but require more effort to do that.

@littledan
Copy link
Member

littledan commented Oct 20, 2020

I'm having trouble understanding. Wouldn't the difference between inherited and allow-multiple be captured by the choice of query algorithm?

@pzuraq
Copy link
Collaborator

pzuraq commented Mar 27, 2022

The latest iteration of the proposal takes into account inheritance in metadata, and the inheritance is described in the README, so I'm closing this issue. If you feel like the README is not descriptive enough, feel free to reopen.

@pzuraq pzuraq closed this as completed Mar 27, 2022
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

No branches or pull requests

3 participants