Skip to content

Commit 23cd385

Browse files
author
Tim Blasi
committed
fix(dart/transform): Handle mixed lifecycle specs
Update the transformer to handle classes which both have a `lifecycle` value and `implement` lifecycle interfaces. Closes angular#3276
1 parent 45b10a1 commit 23cd385

4 files changed

Lines changed: 129 additions & 52 deletions

File tree

‎modules/angular2/src/transform/directive_processor/visitors.dart‎

Lines changed: 103 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ library angular2.transform.directive_processor.visitors;
33
import 'dart:async';
44
import 'package:analyzer/analyzer.dart';
55
import 'package:analyzer/src/generated/java_core.dart';
6+
import 'package:angular2/annotations.dart' show LifecycleEvent;
67
import 'package:angular2/src/render/xhr.dart' show XHR;
78
import 'package:angular2/src/transform/common/annotation_matcher.dart';
89
import 'package:angular2/src/transform/common/async_string_writer.dart';
@@ -216,9 +217,11 @@ class AnnotationsTransformVisitor extends ToSourceVisitor {
216217
final AssetId _assetId;
217218
final bool _inlineViews;
218219
final ConstantEvaluator _evaluator = new ConstantEvaluator();
220+
final Set<String> _ifaceLifecycleEntries = new Set<String>();
221+
bool _isLifecycleWritten = false;
219222
bool _isProcessingView = false;
220223
bool _isProcessingDirective = false;
221-
String _lifecycleValue = null;
224+
String _ifaceLifecyclePrefix = '';
222225

223226
AnnotationsTransformVisitor(AsyncStringWriter writer, this._xhr,
224227
this._annotationMatcher, this._interfaceMatcher, this._assetId,
@@ -231,12 +234,10 @@ class AnnotationsTransformVisitor extends ToSourceVisitor {
231234
/// populates `_lifecycleValue` with the appropriate values if so. If none are
232235
/// present, `_lifecycleValue` is not modified.
233236
void _populateLifecycleValue(ClassDeclaration node) {
234-
var lifecycleEntries = [];
235-
var prefix = '';
236237
var populateImport = (Identifier name) {
237-
if (prefix.isNotEmpty) return;
238+
if (_ifaceLifecyclePrefix.isNotEmpty) return;
238239
var import = _interfaceMatcher.getMatchingImport(name, _assetId);
239-
prefix =
240+
_ifaceLifecyclePrefix =
240241
import != null && import.prefix != null ? '${import.prefix}.' : '';
241242
};
242243

@@ -254,30 +255,32 @@ class AnnotationsTransformVisitor extends ToSourceVisitor {
254255

255256
namesToTest.forEach((name) {
256257
if (_interfaceMatcher.isOnChange(name, _assetId)) {
257-
lifecycleEntries.add('onChange');
258+
_ifaceLifecycleEntries.add('${LifecycleEvent.onChange}');
258259
populateImport(name);
259260
}
260261
if (_interfaceMatcher.isOnDestroy(name, _assetId)) {
261-
lifecycleEntries.add('onDestroy');
262+
_ifaceLifecycleEntries.add('${LifecycleEvent.onDestroy}');
262263
populateImport(name);
263264
}
264265
if (_interfaceMatcher.isOnCheck(name, _assetId)) {
265-
lifecycleEntries.add('onCheck');
266+
_ifaceLifecycleEntries.add('${LifecycleEvent.onCheck}');
266267
populateImport(name);
267268
}
268269
if (_interfaceMatcher.isOnInit(name, _assetId)) {
269-
lifecycleEntries.add('onInit');
270+
_ifaceLifecycleEntries.add('${LifecycleEvent.onInit}');
270271
populateImport(name);
271272
}
272273
if (_interfaceMatcher.isOnAllChangesDone(name, _assetId)) {
273-
lifecycleEntries.add('onAllChangesDone');
274+
_ifaceLifecycleEntries.add('${LifecycleEvent.onAllChangesDone}');
274275
populateImport(name);
275276
}
276277
});
277-
if (lifecycleEntries.isNotEmpty) {
278-
_lifecycleValue = 'const [${prefix}LifecycleEvent.'
279-
'${lifecycleEntries.join(", ${prefix}LifecycleEvent.")}]';
280-
}
278+
}
279+
280+
void _resetState() {
281+
_isLifecycleWritten = _isProcessingView = _isProcessingDirective = false;
282+
_ifaceLifecycleEntries.clear();
283+
_ifaceLifecyclePrefix = '';
281284
}
282285

283286
@override
@@ -294,7 +297,7 @@ class AnnotationsTransformVisitor extends ToSourceVisitor {
294297
}
295298
writer.print(']');
296299

297-
_lifecycleValue = null;
300+
_resetState();
298301
return null;
299302
}
300303

@@ -322,59 +325,110 @@ class AnnotationsTransformVisitor extends ToSourceVisitor {
322325
}
323326
args[i].accept(this);
324327
}
325-
if (_lifecycleValue != null && _isProcessingDirective) {
326-
writer.print(''', lifecycle: $_lifecycleValue ''');
328+
if (!_isLifecycleWritten && _isProcessingDirective) {
329+
var lifecycleValue = _getLifecycleValue();
330+
if (lifecycleValue.isNotEmpty) {
331+
writer.print(', lifecycle: $lifecycleValue');
332+
_isLifecycleWritten = true;
333+
}
327334
}
328335
writer.print(')');
329336
}
330337
return null;
331338
}
332339

340+
String _getLifecycleValue() {
341+
if (_ifaceLifecycleEntries.isNotEmpty) {
342+
var entries = _ifaceLifecycleEntries.toList();
343+
entries.sort();
344+
return 'const [${_ifaceLifecyclePrefix}'
345+
'${entries.join(", ${_ifaceLifecyclePrefix}")}]';
346+
}
347+
return '';
348+
}
349+
333350
/// These correspond to the annotation parameters.
334351
@override
335352
Object visitNamedExpression(NamedExpression node) {
353+
if (!_isProcessingView && !_isProcessingDirective) {
354+
return super.visitNamedExpression(node);
355+
}
336356
// TODO(kegluneq): Remove this limitation.
337-
if (!_isProcessingView ||
338-
node.name is! Label ||
339-
node.name.label is! SimpleIdentifier) {
357+
if (node.name is! Label || node.name.label is! SimpleIdentifier) {
340358
return super.visitNamedExpression(node);
341359
}
342360
var keyString = '${node.name.label}';
343-
if (_inlineViews) {
344-
if (keyString == 'templateUrl') {
345-
// Inline the templateUrl
346-
var url = node.expression.accept(_evaluator);
361+
if (_isProcessingView && _inlineViews) {
362+
var isSuccess = this._inlineView(keyString, node.expression);
363+
if (isSuccess) return null;
364+
}
365+
if (_isProcessingDirective && keyString == 'lifecycle') {
366+
var isSuccess = _populateLifecycleFromNamedExpression(node.expression);
367+
if (isSuccess) {
368+
_isLifecycleWritten = true;
369+
writer.print('lifecycle: ${_getLifecycleValue()}');
370+
return null;
371+
} else {
372+
logger.warning('Failed to parse `lifecycle` value. '
373+
'The following `LifecycleEvent`s may not be called: '
374+
'(${_ifaceLifecycleEntries.join(', ')})');
375+
_isLifecycleWritten = true;
376+
// Do not return -- we will use the default processing here, maintaining
377+
// the original value for `lifecycle`.
378+
}
379+
}
380+
return super.visitNamedExpression(node);
381+
}
382+
383+
/// Populates the lifecycle values from explicitly declared values.
384+
/// Returns whether `node` was successfully processed.
385+
bool _populateLifecycleFromNamedExpression(AstNode node) {
386+
var nodeVal = node.toSource();
387+
for (var evt in LifecycleEvent.values) {
388+
var evtStr = '$evt';
389+
if (nodeVal.contains(evtStr)) {
390+
_ifaceLifecycleEntries.add(evtStr);
391+
}
392+
}
393+
return true;
394+
}
395+
396+
/// Inlines the template and/or style refered to by `keyString`.
397+
/// Returns whether the `keyString` value was successfully processed.
398+
bool _inlineView(String keyString, AstNode node) {
399+
if (keyString == 'templateUrl') {
400+
// Inline the templateUrl
401+
var url = node.accept(_evaluator);
402+
if (url is String) {
403+
writer.print("template: r'''");
404+
writer.asyncPrint(_readOrEmptyString(url));
405+
writer.print("'''");
406+
407+
// We keep the templateUrl in case the body of the template includes
408+
// relative urls that might be inlined later on (e.g. @import
409+
// directives or url() css values in style tags).
410+
writer.print(", templateUrl: r'$url'");
411+
return true;
412+
} else {
413+
logger.warning('template url is not a String $url');
414+
}
415+
} else if (keyString == 'styleUrls') {
416+
// Inline the styleUrls
417+
var urls = node.accept(_evaluator);
418+
writer.print('styles: const [');
419+
for (var url in urls) {
347420
if (url is String) {
348-
writer.print("template: r'''");
421+
writer.print("r'''");
349422
writer.asyncPrint(_readOrEmptyString(url));
350-
writer.print("'''");
351-
352-
// We keep the templateUrl in case the body of the template includes
353-
// relative urls that might be inlined later on (e.g. @import
354-
// directives or url() css values in style tags).
355-
writer.print(", templateUrl: r'$url'");
356-
return null;
423+
writer.print("''', ");
357424
} else {
358-
logger.warning('template url is not a String $url');
359-
}
360-
} else if (keyString == 'styleUrls') {
361-
// Inline the styleUrls
362-
var urls = node.expression.accept(_evaluator);
363-
writer.print('styles: const [');
364-
for (var url in urls) {
365-
if (url is String) {
366-
writer.print("r'''");
367-
writer.asyncPrint(_readOrEmptyString(url));
368-
writer.print("''', ");
369-
} else {
370-
logger.warning('style url is not a String ${url}');
371-
}
425+
logger.warning('style url is not a String ${url}');
372426
}
373-
writer.print(']');
374-
return null;
375427
}
428+
writer.print(']');
429+
return true;
376430
}
377-
return super.visitNamedExpression(node);
431+
return false;
378432
}
379433

380434
/// Attempts to read the content from {@link url}, if it returns null then

‎modules/angular2/test/transform/directive_processor/all_tests.dart‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,9 @@ void _testProcessor(String name, String inputPath,
165165
}
166166
});
167167

168-
if (expectedLogs != null) {
168+
if (expectedLogs == null) {
169+
expect(logger.hasErrors).toBeFalse();
170+
} else {
169171
expect(logger.logs, expectedLogs);
170172
}
171173
});
@@ -180,6 +182,8 @@ class RecordingLogger implements BuildLogger {
180182
@override
181183
final bool convertErrorsToWarnings = false;
182184

185+
bool hasErrors = false;
186+
183187
List<String> logs = [];
184188

185189
void _record(prefix, msg) => logs.add('$prefix: $msg');
@@ -190,7 +194,10 @@ class RecordingLogger implements BuildLogger {
190194

191195
void warning(msg, {AssetId asset, SourceSpan span}) => _record('WARN', msg);
192196

193-
void error(msg, {AssetId asset, SourceSpan span}) => _record('ERROR', msg);
197+
void error(msg, {AssetId asset, SourceSpan span}) {
198+
hasErrors = true;
199+
_record('ERROR', msg);
200+
}
194201

195202
Future writeOutput() => throw new UnimplementedError();
196203
Future addLogFilesFromAsset(AssetId id, [int nextNumber = 1]) =>

‎modules/angular2/test/transform/directive_processor/multiple_interface_lifecycle_files/expected/soup.ng_deps.dart‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,15 @@ void initReflector() {
2222
OnChange,
2323
OnDestroy,
2424
OnInit
25-
]));
25+
]))
26+
..registerType(MixedSoupComponent, new _ngRef.ReflectionInfo(const [
27+
const Component(
28+
selector: '[soup]',
29+
lifecycle: const [LifecycleEvent.onChange, LifecycleEvent.onCheck])
30+
], const [], () => new MixedSoupComponent(), const [OnChange]))
31+
..registerType(MatchedSoupComponent, new _ngRef.ReflectionInfo(const [
32+
const Component(
33+
selector: '[soup]',
34+
lifecycle: const [LifecycleEvent.onChange])
35+
], const [], () => new MatchedSoupComponent(), const [OnChange]));
2636
}

‎modules/angular2/test/transform/directive_processor/multiple_interface_lifecycle_files/soup.dart‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,9 @@ import 'package:angular2/annotations.dart';
44

55
@Component(selector: '[soup]')
66
class MultiSoupComponent implements OnChange, OnDestroy, OnInit {}
7+
8+
@Component(selector: '[soup]', lifecycle: const [LifecycleEvent.onCheck])
9+
class MixedSoupComponent implements OnChange {}
10+
11+
@Component(selector: '[soup]', lifecycle: const [LifecycleEvent.onChange])
12+
class MatchedSoupComponent implements OnChange {}

0 commit comments

Comments
 (0)