Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

Add support for inheritance in Directives and Components #829

Closed
wants to merge 2 commits into from

Conversation

mvuksano
Copy link
Contributor

@mvuksano mvuksano commented Apr 1, 2014

Implements #457

@mhevery
Copy link
Contributor

mhevery commented Apr 1, 2014

return metadata;
}

Iterable<InstanceMirror> _mergeMetadata(Iterable first, Iterable second) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: Refactor this mess

@mhevery
Copy link
Contributor

mhevery commented Apr 7, 2014

This looks like the right approach. Does this cause the transformer tests to fail? If not this means we need to add more tests to the transformer.

Iterable _mergeMetadata(
Iterable superClassMetadataList, Iterable classMetadataList) {

if(superClassMetadataList == null || superClassMetadataList.isEmpty)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit- I believe that this DynamicMetadataExtractor should never return null, so the null superClassMetadataList case and null classMetadataList cases should never be hit.

@blois
Copy link
Contributor

blois commented Apr 9, 2014

It seems that the static route requires that the subclass has an annotation on it, but that the dynamic one does not. Should the subclass always have an annotation?

@mvuksano
Copy link
Contributor Author

mvuksano commented Apr 9, 2014

@blois Yes, I'll fix that. I need to refactor the dynamic generator a bit. Essentially it should not require an annotation on the base class.

@mvuksano mvuksano changed the title [WIP] test(NgDirective): Add test for inheritance Add support for inheritance in Directives and Components Apr 14, 2014
@@ -13,7 +13,7 @@ import 'package:unittest/unittest.dart';

void main() {
describe('expression_extractor', () {
it('should extract all expressions from source and templates', () {
xit('should extract all expressions from source and templates', () {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is broken by ClassElement classElement = clazz.element; in source_metadata_extractor.dart:171. As @blois already said, element is not yet resolved and thus it is null. This works fine when transformers are used as element is resolved, it should only fail if command line (generate-expressions.sh) is used.

This breaks bin/expression_extractor.dart which is invoked from scripts/generate-expressions.sh - @mhevery Is this still used?

@blois
Copy link
Contributor

blois commented Apr 16, 2014

So the command-line extractor is working again? Cool!

Changes look good to me (with one test comment).

@mvuksano
Copy link
Contributor Author

@blois Fixed "iit". Yes, it works (depending on what's your definition of work). - Inheritance will not work if one uses command line generator, but the old functionality is not broken.

This patch supports the following scenario:

```
class Base {
  @Ngattr('foo')
  var foo;
}

@ngdirective(selector:'my-directive')
class MyDirective extends Base {
  @Ngattr('bar')
  var bar;
}
```

MyDirective will now have both properties, foo and bar.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

4 participants