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

feat: Allow using ancestor decorator values to declare descendant decorators #13764

Open
chuckjaz opened this Issue Jan 3, 2017 · 12 comments

Comments

Projects
None yet
7 participants
@chuckjaz
Member

chuckjaz commented Jan 3, 2017

I'm submitting a ... (check one with "x")

[x] feature request

Problem

When specifying a decorator on a component, the descendant decorator overrides the ancestor decorator. This become problematic if the descendant class wants to modify a value that can only be supplied in the @Component decorator. The only solution, currently, is to snap-shot the ancestor values. Snap shots are a maintenance issues because:

  1. If the ancestor values are ever changed as all descendants need to be updated.
  2. Updating all descendant classes is impossible if the component is in a re-usable library as the library author might not have access to all descendants.
  3. It is unclear what the descendant meant to override and what it intended to copy so updating descendant classes is dangerous.

Proposal

Include a special value called inherited that is usable when specifying decorators that allows referencing value from the decorate of the ancestor. inherited would be substitutable for any value in decorator and is substituted, during template code generation, for the ancestor decorator field value for the corresponding field.

The inherited value can be used with object spread syntax to mean include all the ancestor properties with the inherited values. This would be equivalent to specifying all decorator values as inherited.

inherited would also be allowed in providers arrays which is substituted with the ancestor's providers array.

Example

@Component({
  stylesUrl: './base_styles.css',
  providers: [BASE_PROVIDERS]
})
class BaseComponent {
}


@Component({
  ...inherited,
  providers: [inherited, { provider: SomeToken, useValue: someValue} ],
  template: `some template`
})
class MyComponent extends BaseComponent {

}

Limitations

Because the value is not substituted immediately, the value cannot be used in calculations that would otherwise be supported. For example, the following will not be supported:

@Component({
  ...inherited,
  template: inherited + `additional elements` // NOT SUPPORTED
})
class Unsupported extends BaseClass {
}

The above would result in a type error in TypeScript and undefined behavior in ES6. This scenario would need to use an exported constant value that is supported without feature such as,

@Component({
  ...inherited,
  template: BASECLASS_TEMPLATE + `additional elements` // NOT SUPPORTED
})
class Unsupported extends BaseClass {
}

where BASECLASS_TEMPLATE is exported from the base class' module.

@fernandocode

This comment has been minimized.

fernandocode commented Jan 5, 2017

Ok @chuckjaz, this will be very useful and expected.
I just do not know if Include a special value called inherited is the best option to have access to the value of the ancestor component.
I have no knowledge about how angular2 compilation works and its limitations (especially related to compilation OTC (Offline template compiler)). But my suggestion would be to think of some more trivial solution, like the use of functions, similar to the one suggested in this alternative solution to the problem, that following its example would be something similar to this:

@Component({
  stylesUrl: './base_styles.css',
  providers: [BASE_PROVIDERS],
  selector: 'base'
})
class BaseComponent {
}

@Component({
  // ...inherited, // This would not be necessary if all values were already inherited by default, giving the possibility to overwrite or merge.
  //providers: [{ provider: SomeToken, useValue: someValue} ], // <== override
  providers: inheritedValue => [...inheritedValue, { provider: SomeToken, useValue: someValue} ], // <== merge
  template: `some template`,
  //selector: 'sub', // override or using base value
  selector: (parentSelector) => { return parentSelector + 'sub'} // result: basesub
})
class MyComponent extends BaseComponent {

}

Ps: As commented by Günter Zöchbauer in the StackOverflow, OTC could have a problem with that.

@DzmitryShylovich

This comment has been minimized.

Contributor

DzmitryShylovich commented Jan 5, 2017

OTC could have a problem with that.

yeap, u can't use functions in matadata

@fernandocode

This comment has been minimized.

fernandocode commented Jan 5, 2017

@DzmitryShylovich,

By OTC compilation or another reason?
Would not it be possible to change OTC compilation to support this?

In my view as a user angular2 this would be a more elegant and flexible way to use. It is unfortunate that it can not be an option.

@DzmitryShylovich

This comment has been minimized.

Contributor

DzmitryShylovich commented Jan 5, 2017

selector: (parentSelector) => { return parentSelector + 'sub'}

instead of functions I would prefer so something like

selector: '${parent}sub'

and angular during compilation will insert parent value instead of ${parent}

@fernandocode

This comment has been minimized.

fernandocode commented Jan 5, 2017

In your suggestion, Angular2 in compilation would solve:

selector: `${parent}sub` 

for javascript:

selector: function (parent) { return parent + "sub"; };

Or compile it would solve for javascript:

selector: "basesub";

I think the second option would only be supported?

Ps: Sorry if this could be questioning beginners. =/

@chuckjaz

This comment has been minimized.

Member

chuckjaz commented Jan 6, 2017

@fernandocode We don't run any code during AOT. We only use values we can be statically determine from the annotations.

However, I will consider supporting lambda that follows the restrictions of macro functions which would support what you wrote in your example. This means we would still not support:

@Component({
  ...inherited,
  template: inherited + `additional elements` // NOT SUPPORTED
})
class Unsupported extends BaseClass {
}

but will consider supporting doing the above as:

@Component({
  ...inherited,
  template:  inherited => inherited + `additional elements` // SUPPORTED
})
class Unsupported extends BaseClass {
}

Getting the typing of this to be such that TypeScript will correctly flow the type information is a bit tricky and I will give it some more thought.

@aluanhaddad

This comment has been minimized.

aluanhaddad commented Jan 10, 2017

@chuckjaz you efforts are appreciated. Irrespective of my firmly held assertion that it is an undeclared fork of a language and thus disingenuous, and misleading, and badly in need of an actual specification, I find the restrictions that AOT imposes to be crippling at times so anything that would increase expressiveness would be welcome ❤️.

However, I will consider supporting lambda that follows the restrictions of macro functions which would support what you wrote in your example.

What is a TypeScript macro? What is an ECMAScript macro? These languages do not have macros so it sounds like you are now adding new concepts.

With that in mind please try to understand my concern:

I honestly have no desire or intent to offend you with this question, but do you have prior experience designing languages? The TypeScript team has experience designing widely successful languages like C#, Turbo Pascal, and Delphi.

I am also uncomfortable with the design of languages being coupled to web application frameworks, I fear generality will be lost.
To use the example of React, JSX, love it or hate it (I am not a big fan) is specified and maintained independently of React and the respective teams make a concerted effort to maintain that separation. It leads to a better design.

@fernandocode

This comment has been minimized.

fernandocode commented Jan 10, 2017

@aluanhaddad

I do not know if I really understood your position.

But I'd like to better understand some of the points you've mentioned, such as:

  • I do not understand what the term "macro functions" would be in this context, but what is considered to be supported is something the language supports, which was referred to here as "lambda", which is nothing more than "functions" or "Arrows functions" (which would be a simplified way to declare a function, similar to lambda C#), what's the problem with that?
  • Another point would be:

I am also uncomfortable with the design of languages being coupled to web application frameworks, I fear generality will be lost.

I think this will not increase the coupling between the framework and the language, since this will in the end be javascript and what is being called lambda support will actually support javascript functions.

I believe that this is a real problem and that it hinders the day to day development of applications, I do not know if I have a different view or if the way work is not correct but this is a problem that should receive more attention. And if all this being proposed is not appropriate, I think we should take a step back and review the use of annotations in the framework.

@chuckjaz

This comment has been minimized.

Member

chuckjaz commented Jan 10, 2017

@aluanhaddad I appreciate that this is under-document. I am working on trying to document what can be evaluated during metadata collection and this is a start but certainly not sufficient.

More specifically, a macro function is a subset of JavaScript functions the collector supports evaluating to produce metadata. It currently only supports a function that contains a single return statement (or a non-block lambda which is short-hand for the same thing).

As for my resume, I have considerable experience in theoretic and practical language design as well as how such features affect the a library using these language, specifically I had significant influence on Delphi with VCL, Turbo Pascal with Turbo Vision and OWL, and a minor effect on C# and significant impact on WPF (XAML). I was part of the original TypeScript team (before leaving Microsoft).

I decline to debate the relative merits of our general approach in this issue but feel free to take it up with me in a more appropriate forum. I am always up for a good language discussion!

@aluanhaddad

This comment has been minimized.

aluanhaddad commented Jan 11, 2017

@chuckjaz Thank you for your response. I very much appreciate your credentials. They are first class and clearly you know what you are doing.

I have written some WPF applications and I found XAML to be a joy to work with.

Your qualifications are at the same time generally exceptional and uniquely apropos to the work you are currently doing on Angular 2.

I also want to thank you for the link to your documentation of the Angular 2 metadata collection process. I appreciate it and look forward to seeing it evolve.

@wardbell

This comment has been minimized.

Contributor

wardbell commented Jan 13, 2017

@chuckjaz Started playing with tactics for @Component metadata "inheritance" with the current release (2.4.0).

Tried various stunts. You can see the JIT version in this plunker: http://plnkr.co/edit/33UgUQHUv2v0u3Y2to9X?p=preview

Clearly it can work. I'd like it to work along the lines you propose in this PR so that I don't have to do the madness you see in app.sub.component.ts.

Thanks. Happy to chat directly any time.

@vinagreti

This comment has been minimized.

Contributor

vinagreti commented Mar 29, 2017

Any news about this issue? Is there any support for @decorators inheritance? I'm trying to use the "animations" property from a base @component in a child that extends it. My task is make a animated component and extend it in other components that need to be animated so I don't repeat myself. Is it possible already or will be in the future?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment