Skip to content

Commit 2604402

Browse files
author
Tim Blasi
committed
feat(dart/transform): Parse directives dependencies from the Dart ast
Previously, we parsed dependencies out of a the stringified value of `directives`, which is brittle and error-prone. Move this parsing into `DirectiveProcessor` where we have the full Dart ast to help.
1 parent ca5e31b commit 2604402

9 files changed

Lines changed: 221 additions & 135 deletions

File tree

‎modules_dart/transform/lib/src/transform/common/code/reflection_info_code.dart‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor<ReflectionInfoModel> {
8383

8484
if (node.metadata != null) {
8585
node.metadata.forEach((node) {
86+
final extractedDirectives = _extractDirectives(node);
87+
if (extractedDirectives != null) {
88+
model.directives.addAll(extractedDirectives);
89+
}
8690
model.annotations.add(_annotationVisitor.visitAnnotation(node));
8791
});
8892
}
@@ -137,6 +141,36 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor<ReflectionInfoModel> {
137141
}
138142
}
139143

144+
Iterable<PrefixedDirective> _extractDirectives(Annotation node) {
145+
var shouldProcess = _annotationMatcher.isComponent(node, assetId);
146+
shouldProcess = shouldProcess || _annotationMatcher.isView(node, assetId);
147+
148+
if (node.arguments == null && node.arguments.arguments == null) return null;
149+
final directivesNode = node.arguments.arguments.firstWhere((arg) {
150+
return arg is NamedExpression && '${arg.name.label}' == 'directives';
151+
}, orElse: () => null);
152+
if (directivesNode == null) return null;
153+
154+
if (directivesNode.expression is! ListLiteral) {
155+
logger.warning('Angular 2 expects a list literal for `directives` '
156+
'but found a ${directivesNode.expression.runtimeType}');
157+
return null;
158+
}
159+
final directives = <PrefixedDirective>[];
160+
for (var dep in (directivesNode.expression as ListLiteral).elements) {
161+
if (dep is PrefixedIdentifier) {
162+
directives.add(new PrefixedDirective()
163+
..prefix = '${dep.prefix}'
164+
..name = '${dep.identifier}');
165+
} else if (dep is Identifier) {
166+
directives.add(new PrefixedDirective()..name = '${dep}');
167+
} else {
168+
logger.warning('Found unexpected value $dep in `directives`.');
169+
}
170+
}
171+
return directives;
172+
}
173+
140174
@override
141175
ReflectionInfoModel visitFunctionDeclaration(FunctionDeclaration node) {
142176
if (!node.metadata

‎modules_dart/transform/lib/src/transform/common/model/reflection_info_model.pb.dart‎

Lines changed: 70 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎modules_dart/transform/lib/src/transform/common/model/reflection_info_model.proto‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,15 @@ message PropertyMetadataModel {
1313
repeated AnnotationModel annotations = 2;
1414
}
1515

16+
message PrefixedDirective {
17+
// The prefix used to reference this Directive, if any.
18+
optional string prefix = 1;
19+
20+
// The name of the Directive or directive alias.
21+
// See https://goo.gl/d8XPt0 for info on directive aliases.
22+
optional string name = 2;
23+
}
24+
1625
message ReflectionInfoModel {
1726
// The (potentially prefixed) name of this Injectable.
1827
// This can be a `Type` or a function name.
@@ -32,4 +41,8 @@ message ReflectionInfoModel {
3241

3342
// Entries for all properties with associated metadata.
3443
repeated PropertyMetadataModel propertyMetadata = 7;
44+
45+
// Directive dependencies parsed from the @View or @Component `directives`
46+
// parameter.
47+
repeated PrefixedDirective directives = 8;
3548
}

‎modules_dart/transform/lib/src/transform/template_compiler/compile_data_creator.dart‎

Lines changed: 5 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -38,58 +38,6 @@ class CompileDataResults {
3838
this.ngMeta, this.viewDefinitions, this.directiveMetadatas);
3939
}
4040

41-
class _PrefixedDirectiveName {
42-
final String prefix;
43-
final String directiveName;
44-
45-
_PrefixedDirectiveName(String prefix, this.directiveName)
46-
: this.prefix = prefix == null ? '' : prefix;
47-
48-
@override
49-
String toString() {
50-
if (prefix.isEmpty) {
51-
return directiveName;
52-
} else {
53-
return '${prefix}.${directiveName}';
54-
}
55-
}
56-
}
57-
58-
String _directivesValue(ReflectionInfoModel model, bool test(AnnotationModel)) {
59-
final viewAnnotation = model.annotations.firstWhere(test, orElse: () => null);
60-
if (viewAnnotation == null) return null;
61-
final directivesParam = viewAnnotation.namedParameters
62-
.firstWhere((p) => p.name == 'directives', orElse: () => null);
63-
return directivesParam != null ? directivesParam.value : null;
64-
}
65-
66-
// TODO(kegluneq): Parse this value when creating [NgDepsModel]?
67-
/// Find the `directives` parameter on the @View annotation and make sure that
68-
/// all necessary [CompileDirectiveMetadata] objects are available.
69-
Iterable<_PrefixedDirectiveName> _getDirectiveDeps(ReflectionInfoModel model) {
70-
var directives = _directivesValue(model, (m) => m.isView);
71-
if (directives == null) {
72-
directives = _directivesValue(model, (m) => m.isComponent);
73-
}
74-
if (directives == null) return const [];
75-
directives = directives.trim();
76-
for (var toTrim in ['const [', '[']) {
77-
if (directives.startsWith(toTrim) && directives.endsWith(']')) {
78-
directives =
79-
directives.substring(toTrim.length, directives.length - 1).trim();
80-
}
81-
}
82-
if (directives.length == 0) return const [];
83-
return directives.split(',').map((p) {
84-
var parts = p.trim().split('.');
85-
if (parts.length == 1) {
86-
return new _PrefixedDirectiveName(null, parts[0]);
87-
} else {
88-
return new _PrefixedDirectiveName(parts[0], parts[1]);
89-
}
90-
});
91-
}
92-
9341
/// Creates [ViewDefinition] objects for all `View` `Directive`s defined in
9442
/// `entryPoint`.
9543
class _CompileDataCreator {
@@ -127,7 +75,7 @@ class _CompileDataCreator {
12775
if (compileDirectiveMetadata.template != null) {
12876
final compileDatum = new NormalizedComponentWithViewDirectives(
12977
compileDirectiveMetadata, <CompileDirectiveMetadata>[]);
130-
for (var dep in _getDirectiveDeps(reflectable)) {
78+
for (var dep in reflectable.directives) {
13179
if (!ngMetaMap.containsKey(dep.prefix)) {
13280
logger.warning(
13381
'Missing prefix "${dep.prefix}" '
@@ -137,11 +85,10 @@ class _CompileDataCreator {
13785
}
13886
final depNgMeta = ngMetaMap[dep.prefix];
13987

140-
if (depNgMeta.types.containsKey(dep.directiveName)) {
141-
compileDatum.directives.add(depNgMeta.types[dep.directiveName]);
142-
} else if (depNgMeta.aliases.containsKey(dep.directiveName)) {
143-
compileDatum.directives
144-
.addAll(depNgMeta.flatten(dep.directiveName));
88+
if (depNgMeta.types.containsKey(dep.name)) {
89+
compileDatum.directives.add(depNgMeta.types[dep.name]);
90+
} else if (depNgMeta.aliases.containsKey(dep.name)) {
91+
compileDatum.directives.addAll(depNgMeta.flatten(dep.name));
14592
} else {
14693
logger.warning('Could not find Directive entry for $dep. '
14794
'Please be aware that Dart transformers have limited support for '

‎modules_dart/transform/test/transform/directive_processor/all_tests.dart‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import 'package:angular2/src/transform/common/annotation_matcher.dart';
1010
import 'package:angular2/src/transform/common/code/ng_deps_code.dart';
1111
import 'package:angular2/src/transform/common/asset_reader.dart';
1212
import 'package:angular2/src/transform/common/logging.dart' as log;
13+
import 'package:angular2/src/transform/common/model/ng_deps_model.pb.dart';
1314
import 'package:angular2/src/transform/common/model/reflection_info_model.pb.dart';
1415
import 'package:angular2/src/transform/common/ng_meta.dart';
1516
import 'package:barback/barback.dart';
@@ -441,6 +442,58 @@ void allTests() {
441442
.toContain('[soup]');
442443
});
443444
});
445+
446+
describe('directives', () {
447+
final reflectableNamed = (NgDepsModel model, String name) {
448+
return model.reflectables
449+
.firstWhere((r) => r.name == name, orElse: () => null);
450+
};
451+
452+
it('should populate `directives` from @View value specified second.',
453+
() async {
454+
var model =
455+
(await _testCreateModel('directives_files/components.dart')).ngDeps;
456+
final componentFirst = reflectableNamed(model, 'ComponentFirst');
457+
expect(componentFirst).toBeNotNull();
458+
expect(componentFirst.directives).toBeNotNull();
459+
expect(componentFirst.directives.length).toEqual(2);
460+
expect(componentFirst.directives.first)
461+
.toEqual(new PrefixedDirective()..name = 'Dep');
462+
expect(componentFirst.directives[1]).toEqual(new PrefixedDirective()
463+
..name = 'Dep'
464+
..prefix = 'dep2');
465+
});
466+
467+
it('should populate `directives` from @View value specified first.',
468+
() async {
469+
var model =
470+
(await _testCreateModel('directives_files/components.dart')).ngDeps;
471+
final viewFirst = reflectableNamed(model, 'ViewFirst');
472+
expect(viewFirst).toBeNotNull();
473+
expect(viewFirst.directives).toBeNotNull();
474+
expect(viewFirst.directives.length).toEqual(2);
475+
expect(viewFirst.directives.first).toEqual(new PrefixedDirective()
476+
..name = 'Dep'
477+
..prefix = 'dep2');
478+
expect(viewFirst.directives[1])
479+
.toEqual(new PrefixedDirective()..name = 'Dep');
480+
});
481+
482+
it('should populate `directives` from @Component value with no @View.',
483+
() async {
484+
var model =
485+
(await _testCreateModel('directives_files/components.dart')).ngDeps;
486+
final componentOnly = reflectableNamed(model, 'ComponentOnly');
487+
expect(componentOnly).toBeNotNull();
488+
expect(componentOnly.directives).toBeNotNull();
489+
expect(componentOnly.directives.length).toEqual(2);
490+
expect(componentOnly.directives.first)
491+
.toEqual(new PrefixedDirective()..name = 'Dep');
492+
expect(componentOnly.directives[1]).toEqual(new PrefixedDirective()
493+
..name = 'Dep'
494+
..prefix = 'dep2');
495+
});
496+
});
444497
}
445498

446499
Future<NgMeta> _testCreateModel(String inputPath,

‎modules_dart/transform/test/transform/directive_processor/directives_files/components.dart‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
library angular2.test.transform.directive_processor.directive_files.components;
2+
3+
import 'package:angular2/angular2.dart'
4+
show Component, Directive, View, NgElement;
5+
import 'dep1.dart';
6+
import 'dep2.dart' as dep2;
7+
8+
@Component(selector: 'component-first')
9+
@View(template: '<dep1></dep1><dep2></dep2>', directives: [Dep, dep2.Dep])
10+
class ComponentFirst {}
11+
12+
@View(template: '<dep1></dep1><dep2></dep2>', directives: [dep2.Dep, Dep])
13+
@Component(selector: 'view-first')
14+
class ViewFirst {}
15+
16+
@Component(
17+
selector: 'component-only',
18+
template: '<dep1></dep1><dep2></dep2>',
19+
directives: [Dep, dep2.Dep])
20+
class ComponentOnly {}

‎modules_dart/transform/test/transform/directive_processor/directives_files/dep1.dart‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
library angular2.test.transform.directive_processor.directive_files.dep1;
2+
3+
import 'package:angular2/angular2.dart'
4+
show Component, Directive, View, NgElement;
5+
6+
@Component(selector: 'dep1')
7+
@View(template: 'Dep1')
8+
class Dep {}

‎modules_dart/transform/test/transform/directive_processor/directives_files/dep2.dart‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
library angular2.test.transform.directive_processor.directive_files.dep2;
2+
3+
import 'package:angular2/angular2.dart'
4+
show Component, Directive, View, NgElement;
5+
6+
@Component(selector: 'dep2')
7+
@View(template: 'Dep2')
8+
class Dep {}

0 commit comments

Comments
 (0)