X Tutup
Skip to content

Commit d1b35f9

Browse files
author
Tim Blasi
committed
fix(dart/transform): Don't set ReflectionCapabilities over an async gap
Update the transformer's `TemplateCompiler` phase to avoid setting `reflector.reflectionCapabilities`, allowing asynchronous operations, and restoring the original value, which allows `reflector.reflectionCapabilities` to get into a bad state.
1 parent f3dd9b5 commit d1b35f9

File tree

4 files changed

+64
-50
lines changed

4 files changed

+64
-50
lines changed

‎modules/angular2/src/transform/template_compiler/generator.dart‎

Lines changed: 33 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ import 'package:angular2/src/change_detection/parser/lexer.dart' as ng;
66
import 'package:angular2/src/change_detection/parser/parser.dart' as ng;
77
import 'package:angular2/src/core/compiler/proto_view_factory.dart';
88
import 'package:angular2/src/render/api.dart';
9-
import 'package:angular2/src/render/dom/compiler/compiler.dart';
9+
import 'package:angular2/src/render/dom/compiler/compile_pipeline.dart';
1010
import 'package:angular2/src/render/dom/compiler/template_loader.dart';
11+
import 'package:angular2/src/render/dom/view/property_setter_factory.dart';
1112
import 'package:angular2/src/services/xhr.dart' show XHR;
1213
import 'package:angular2/src/reflection/reflection.dart';
1314
import 'package:angular2/src/services/url_resolver.dart';
@@ -17,7 +18,7 @@ import 'package:barback/barback.dart';
1718

1819
import 'change_detector_codegen.dart' as change;
1920
import 'compile_step_factory.dart';
20-
import 'recording_reflection_capabilities.dart';
21+
import 'reflection_capabilities.dart';
2122
import 'reflector_register_codegen.dart' as reg;
2223
import 'view_definition_creator.dart';
2324

@@ -39,34 +40,21 @@ Future<String> processTemplates(AssetReader reader, AssetId entryPoint,
3940
var result = await extractor.extractTemplates(viewDefEntry.viewDef);
4041
if (result == null) continue;
4142

42-
registrations.generate(result.recording);
43+
if (generateRegistrations) {
44+
// TODO(kegluneq): Generate these getters & setters based on the
45+
// `ProtoViewDto` rather than querying the `ReflectionCapabilities`.
46+
registrations.generate(result.recording);
47+
}
4348
if (result.protoView != null && generateChangeDetectors) {
44-
var savedReflectionCapabilities = reflector.reflectionCapabilities;
45-
var recordingCapabilities = new RecordingReflectionCapabilities();
46-
reflector.reflectionCapabilities = recordingCapabilities;
47-
49+
var saved = reflector.reflectionCapabilities;
50+
reflector.reflectionCapabilities = const NullReflectionCapabilities();
4851
var defs = getChangeDetectorDefinitions(viewDefEntry.hostMetadata,
4952
result.protoView, viewDefEntry.viewDef.directives);
5053
for (var i = 0; i < defs.length; ++i) {
5154
changeDetectorClasses.generate('${rType.typeName}',
5255
'_${rType.typeName}_ChangeDetector$i', defs[i]);
5356
}
54-
55-
// Check that getters, setters, methods are the same as above.
56-
assert(recordingCapabilities.getterNames
57-
.containsAll(result.recording.getterNames));
58-
assert(result.recording.getterNames
59-
.containsAll(recordingCapabilities.getterNames));
60-
assert(recordingCapabilities.setterNames
61-
.containsAll(result.recording.setterNames));
62-
assert(result.recording.setterNames
63-
.containsAll(recordingCapabilities.setterNames));
64-
assert(recordingCapabilities.methodNames
65-
.containsAll(result.recording.methodNames));
66-
assert(result.recording.methodNames
67-
.containsAll(recordingCapabilities.methodNames));
68-
69-
reflector.reflectionCapabilities = savedReflectionCapabilities;
57+
reflector.reflectionCapabilities = saved;
7058
}
7159
}
7260

@@ -88,25 +76,40 @@ Future<String> processTemplates(AssetReader reader, AssetId entryPoint,
8876
/// template code if necessary, and determines what values will be
8977
/// reflectively accessed from that template.
9078
class _TemplateExtractor {
91-
final RenderCompiler _compiler;
79+
final CompileStepFactory _factory;
80+
final TemplateLoader _loader;
9281

93-
_TemplateExtractor(XHR xhr) : _compiler = new DomCompiler(
94-
new CompileStepFactory(new ng.Parser(new ng.Lexer())),
95-
new TemplateLoader(xhr, new UrlResolver()));
82+
_TemplateExtractor(XHR xhr)
83+
: _factory = new CompileStepFactory(new ng.Parser(new ng.Lexer())),
84+
_loader = new TemplateLoader(xhr, new UrlResolver());
9685

9786
Future<_ExtractResult> extractTemplates(ViewDefinition viewDef) async {
9887
// Check for "imperative views".
9988
if (viewDef.template == null && viewDef.absUrl == null) return null;
10089

90+
var templateEl = await _loader.load(viewDef);
91+
92+
// NOTE(kegluneq): Since this is a global, we must not have any async
93+
// operations between saving and restoring it, otherwise we can get into
94+
// a bad state. See issue #2359 for additional context.
10195
var savedReflectionCapabilities = reflector.reflectionCapabilities;
10296
var recordingCapabilities = new RecordingReflectionCapabilities();
10397
reflector.reflectionCapabilities = recordingCapabilities;
10498

105-
// TODO(kegluneq): Rewrite url to inline `template` where possible.
106-
var protoViewDto = await _compiler.compile(viewDef);
99+
var subtaskPromises = [];
100+
var pipeline =
101+
new CompilePipeline(_factory.createSteps(viewDef, subtaskPromises));
102+
103+
var compileElements = pipeline.process(
104+
templateEl, ProtoViewDto.COMPONENT_VIEW_TYPE, viewDef.componentId);
105+
var protoViewDto = compileElements[0].inheritedProtoView
106+
.build(new PropertySetterFactory());
107107

108108
reflector.reflectionCapabilities = savedReflectionCapabilities;
109-
return new _ExtractResult(recordingCapabilities, protoViewDto);
109+
110+
return Future
111+
.wait(subtaskPromises)
112+
.then((_) => new _ExtractResult(recordingCapabilities, protoViewDto));
110113
}
111114
}
112115

modules/angular2/src/transform/template_compiler/recording_reflection_capabilities.dart renamed to modules/angular2/src/transform/template_compiler/reflection_capabilities.dart

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,12 @@
1-
library angular2.transform.template_compiler.recording_reflection_capabilities;
1+
library angular2.transform.template_compiler.reflection_capabilities;
22

33
import 'package:angular2/src/reflection/reflection_capabilities.dart';
44
import 'package:angular2/src/reflection/types.dart';
55

6-
/// ReflectionCapabilities object that records requests for `getter`s,
7-
/// `setter`s, and `method`s so these can be code generated rather than
8-
/// reflectively accessed at runtime.
9-
class RecordingReflectionCapabilities implements ReflectionCapabilities {
10-
/// The names of all requested `getter`s.
11-
final Set<String> getterNames = new Set<String>();
12-
/// The names of all requested `setter`s.
13-
final Set<String> setterNames = new Set<String>();
14-
/// The names of all requested `method`s.
15-
final Set<String> methodNames = new Set<String>();
6+
/// ReflectionCapabilities object that responds to all requests for `getter`s,
7+
/// `setter`s, and `method`s with `null`.
8+
class NullReflectionCapabilities implements ReflectionCapabilities {
9+
const NullReflectionCapabilities();
1610

1711
_notImplemented(String name) => throw 'Not implemented: $name';
1812

@@ -24,22 +18,40 @@ class RecordingReflectionCapabilities implements ReflectionCapabilities {
2418

2519
List annotations(typeOrFunc) => _notImplemented('annotations');
2620

21+
GetterFn getter(String name) => _nullGetter;
22+
23+
SetterFn setter(String name) => _nullSetter;
24+
25+
MethodFn method(String name) => _nullMethod;
26+
}
27+
28+
_nullGetter(Object p) => null;
29+
_nullSetter(Object p, v) => null;
30+
_nullMethod(Object p, List a) => null;
31+
32+
/// ReflectionCapabilities object that records requests for `getter`s,
33+
/// `setter`s, and `method`s so these can be code generated rather than
34+
/// reflectively accessed at runtime.
35+
class RecordingReflectionCapabilities extends NullReflectionCapabilities {
36+
/// The names of all requested `getter`s.
37+
final Set<String> getterNames = new Set<String>();
38+
/// The names of all requested `setter`s.
39+
final Set<String> setterNames = new Set<String>();
40+
/// The names of all requested `method`s.
41+
final Set<String> methodNames = new Set<String>();
42+
2743
GetterFn getter(String name) {
2844
getterNames.add(name);
29-
return _nullGetter;
45+
return super.getter(name);
3046
}
3147

3248
SetterFn setter(String name) {
3349
setterNames.add(name);
34-
return _nullSetter;
50+
return super.setter(name);
3551
}
3652

3753
MethodFn method(String name) {
3854
methodNames.add(name);
39-
return _nullMethod;
55+
return super.method(name);
4056
}
4157
}
42-
43-
_nullGetter(Object p) => null;
44-
_nullSetter(Object p, v) => null;
45-
_nullMethod(Object p, List a) => null;

‎modules/angular2/src/transform/template_compiler/reflector_register_codegen.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ library angular2.transform.template_compiler.reflector_register_codegen;
22

33
import 'package:angular2/src/transform/common/names.dart';
44
import 'package:angular2/src/transform/common/property_utils.dart' as prop;
5-
import 'recording_reflection_capabilities.dart';
5+
import 'reflection_capabilities.dart';
66

77
class Codegen {
88
final StringBuffer _buf = new StringBuffer();

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,6 @@ void allTests() {
103103
output = await process(new AssetId('a', inputPath));
104104
_formatThenExpectEquals(output, expected);
105105
});
106-
107106
});
108107
}
109108

0 commit comments

Comments
 (0)
X Tutup