X Tutup
Skip to content

Commit bdd031a

Browse files
author
Tim Blasi
committed
feat(dart/transform): Match runtime semantics for template values
Match [ViewResolver][]'s semantics for reading template and style values from `@Component` and `@View` annotations. We now warn if template and/or style values appear on an `@Component` annotation and a `@View` annotation is present. [ViewResolver]: https://github.com/angular/angular/blob/7c6130c2c5d8ac2d5ba8df5f70b7f1264c283a2c/modules/angular2/src/core/linker/view_resolver.ts
1 parent 798be27 commit bdd031a

File tree

6 files changed

+129
-22
lines changed

6 files changed

+129
-22
lines changed

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

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,6 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor<ReflectionInfoModel> {
2525
final ParameterVisitor _parameterVisitor = new ParameterVisitor();
2626
final _PropertyMetadataVisitor _propMetadataVisitor;
2727

28-
/// Whether an Angular 2 `Reflection` has been found.
29-
bool _foundNgReflection = false;
30-
3128
ReflectionInfoVisitor._(this.assetId, this._annotationMatcher,
3229
this._annotationVisitor, this._propMetadataVisitor);
3330

@@ -38,8 +35,6 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor<ReflectionInfoModel> {
3835
annotationVisitor, new _PropertyMetadataVisitor(annotationVisitor));
3936
}
4037

41-
bool get shouldCreateNgDeps => _foundNgReflection;
42-
4338
ConstructorDeclaration _getCtor(ClassDeclaration node) {
4439
int numCtorsFound = 0;
4540
var ctor = null;
@@ -82,13 +77,26 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor<ReflectionInfoModel> {
8277
}
8378

8479
if (node.metadata != null) {
80+
var componentDirectives, viewDirectives;
8581
node.metadata.forEach((node) {
86-
final extractedDirectives = _extractDirectives(node);
87-
if (extractedDirectives != null) {
88-
model.directives.addAll(extractedDirectives);
82+
if (_annotationMatcher.isComponent(node, assetId)) {
83+
componentDirectives = _extractDirectives(node);
84+
} else if (_annotationMatcher.isView(node, assetId)) {
85+
viewDirectives = _extractDirectives(node);
8986
}
9087
model.annotations.add(_annotationVisitor.visitAnnotation(node));
9188
});
89+
if (componentDirectives != null && componentDirectives.isNotEmpty) {
90+
if (viewDirectives != null) {
91+
logger.warning(
92+
'Cannot specify view parameters on @Component when a @View '
93+
'is present. Component name: ${model.name}',
94+
asset: assetId);
95+
}
96+
model.directives.addAll(componentDirectives);
97+
} else if (viewDirectives != null) {
98+
model.directives.addAll(viewDirectives);
99+
}
92100
}
93101
if (ctor != null &&
94102
ctor.parameters != null &&
@@ -141,20 +149,26 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor<ReflectionInfoModel> {
141149
}
142150
}
143151

152+
/// Returns [PrefixedDirective] values parsed from the value of the
153+
/// `directives` parameter of the provided `node`.
154+
/// This will always return a non-null value, so if there are no `directives`
155+
/// specified on `node`, it will return an empty iterable.
144156
Iterable<PrefixedDirective> _extractDirectives(Annotation node) {
145-
var shouldProcess = _annotationMatcher.isComponent(node, assetId);
146-
shouldProcess = shouldProcess || _annotationMatcher.isView(node, assetId);
157+
assert(_annotationMatcher.isComponent(node, assetId) ||
158+
_annotationMatcher.isView(node, assetId));
147159

148-
if (node.arguments == null && node.arguments.arguments == null) return null;
160+
if (node.arguments == null && node.arguments.arguments == null) {
161+
return const [];
162+
}
149163
final directivesNode = node.arguments.arguments.firstWhere((arg) {
150164
return arg is NamedExpression && '${arg.name.label}' == 'directives';
151165
}, orElse: () => null);
152-
if (directivesNode == null) return null;
166+
if (directivesNode == null) return const [];
153167

154168
if (directivesNode.expression is! ListLiteral) {
155169
logger.warning('Angular 2 expects a list literal for `directives` '
156170
'but found a ${directivesNode.expression.runtimeType}');
157-
return null;
171+
return const [];
158172
}
159173
final directives = <PrefixedDirective>[];
160174
for (var dep in (directivesNode.expression as ListLiteral).elements) {

‎modules_dart/transform/lib/src/transform/common/directive_metadata_reader.dart‎

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import 'package:angular2/src/core/linker/interfaces.dart' show LifecycleHooks;
1010
import 'package:angular2/src/core/metadata/view.dart' show ViewEncapsulation;
1111
import 'package:angular2/src/transform/common/annotation_matcher.dart';
1212
import 'package:angular2/src/transform/common/interface_matcher.dart';
13+
import 'package:angular2/src/transform/common/logging.dart';
1314
import 'package:barback/barback.dart' show AssetId;
1415

1516
import 'naive_eval.dart';
@@ -173,6 +174,21 @@ class _DirectiveMetadataVisitor extends Object
173174
template: _template);
174175
}
175176

177+
/// Ensures that we do not specify View values on an `@Component` annotation
178+
/// when there is a @View annotation present.
179+
void _validateTemplates() {
180+
if (_cmpTemplate != null && _viewTemplate != null) {
181+
var name = '(Unknown)';
182+
if (_type != null && _type.name != null && _type.name.isNotEmpty) {
183+
name = _type.name;
184+
}
185+
logger.warning(
186+
'Cannot specify view parameters on @Component when a @View '
187+
'is present. Component name: ${name}',
188+
asset: _assetId);
189+
}
190+
}
191+
176192
@override
177193
Object visitAnnotation(Annotation node) {
178194
var isComponent = _annotationMatcher.isComponent(node, _assetId);
@@ -187,21 +203,22 @@ class _DirectiveMetadataVisitor extends Object
187203
_isComponent = isComponent;
188204
_hasMetadata = true;
189205
if (isComponent) {
190-
var t = new _CompileTemplateMetadataVisitor().visitAnnotation(node);
191-
if (t.template != null || t.templateUrl != null) {
192-
_cmpTemplate = t;
193-
}
206+
_cmpTemplate =
207+
new _CompileTemplateMetadataVisitor().visitAnnotation(node);
208+
_validateTemplates();
194209
}
195210
super.visitAnnotation(node);
196211
} else if (_annotationMatcher.isView(node, _assetId)) {
197212
if (_viewTemplate != null) {
213+
// TODO(kegluneq): Support multiple views on a single class.
198214
throw new FormatException(
199215
'Only one View is allowed per class. '
200216
'Found unexpected "$node".',
201217
'$node' /* source */);
202218
}
203219
_viewTemplate =
204220
new _CompileTemplateMetadataVisitor().visitAnnotation(node);
221+
_validateTemplates();
205222
}
206223

207224
// Annotation we do not recognize - no need to visit.
@@ -353,16 +370,24 @@ class _LifecycleHookVisitor extends SimpleAstVisitor<List<LifecycleHooks>> {
353370
/// [CompileTemplateMetadata].
354371
class _CompileTemplateMetadataVisitor
355372
extends RecursiveAstVisitor<CompileTemplateMetadata> {
356-
ViewEncapsulation _encapsulation = ViewEncapsulation.Emulated;
357-
String _template = null;
358-
String _templateUrl = null;
359-
List<String> _styles = null;
360-
List<String> _styleUrls = null;
373+
ViewEncapsulation _encapsulation;
374+
String _template;
375+
String _templateUrl;
376+
List<String> _styles;
377+
List<String> _styleUrls;
361378

362379
@override
363380
CompileTemplateMetadata visitAnnotation(Annotation node) {
364381
super.visitAnnotation(node);
365382

383+
if (_encapsulation == null &&
384+
_template == null &&
385+
_templateUrl == null &&
386+
_styles == null &&
387+
_styleUrls == null) {
388+
return null;
389+
}
390+
366391
return new CompileTemplateMetadata(
367392
encapsulation: _encapsulation,
368393
template: _template,

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,44 @@ void allTests() {
493493
..name = 'Dep'
494494
..prefix = 'dep2');
495495
});
496+
497+
it('should warn if @Component has a `template` and @View is present.',
498+
() async {
499+
final logger = new RecordingLogger();
500+
final model = await _testCreateModel('bad_directives_files/template.dart',
501+
logger: logger);
502+
var warning =
503+
logger.logs.firstWhere((l) => l.contains('WARN'), orElse: () => null);
504+
expect(warning).toBeNotNull();
505+
expect(warning.toLowerCase())
506+
.toContain('cannot specify view parameters on @component');
507+
});
508+
509+
it('should warn if @Component has a `templateUrl` and @View is present.',
510+
() async {
511+
final logger = new RecordingLogger();
512+
final model = await _testCreateModel(
513+
'bad_directives_files/template_url.dart',
514+
logger: logger);
515+
var warning =
516+
logger.logs.firstWhere((l) => l.contains('WARN'), orElse: () => null);
517+
expect(warning).toBeNotNull();
518+
expect(warning.toLowerCase())
519+
.toContain('cannot specify view parameters on @component');
520+
});
521+
522+
it('should warn if @Component has `directives` and @View is present.',
523+
() async {
524+
final logger = new RecordingLogger();
525+
final model = await _testCreateModel(
526+
'bad_directives_files/directives.dart',
527+
logger: logger);
528+
var warning =
529+
logger.logs.firstWhere((l) => l.contains('WARN'), orElse: () => null);
530+
expect(warning).toBeNotNull();
531+
expect(warning.toLowerCase())
532+
.toContain('cannot specify view parameters on @component');
533+
});
496534
});
497535
}
498536

‎modules_dart/transform/test/transform/directive_processor/bad_directives_files/directives.dart‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
library angular2.test.transform.directive_processor.bad_directives_files.directives;
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', directives: [Dep, dep2.Dep])
9+
@View(template: '<div>Hi</div>')
10+
class BadDirectives {}

‎modules_dart/transform/test/transform/directive_processor/bad_directives_files/template.dart‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
library angular2.test.transform.directive_processor.bad_directives_files.template;
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', template: 'bad!')
9+
@View(template: 'good!', directives: [Dep, dep2.Dep])
10+
class BadTemplate {}

‎modules_dart/transform/test/transform/directive_processor/bad_directives_files/template_url.dart‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
library angular2.test.transform.directive_processor.bad_directives_files.template_url;
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', templateUrl: 'bad_template.html')
9+
@View(templateUrl: 'good_template.html', directives: [Dep, dep2.Dep])
10+
class BadTemplateUrl {}

0 commit comments

Comments
 (0)
X Tutup