From 97d24563f492b5cea29e8da3e8e41c69d3d10c95 Mon Sep 17 00:00:00 2001 From: Tim Blasi Date: Fri, 8 May 2015 17:29:21 -0700 Subject: [PATCH] feat(dart/transform): Inline `templateUrl` values Modify DirectiveProcessor to inline `templateUrl` values to avoid making additional browser requests. Closes #1035 --- .../transform/common/async_string_writer.dart | 75 +++++++++++++ .../xhr_impl.dart | 0 .../directive_processor/rewriter.dart | 28 +++-- .../directive_processor/transformer.dart | 5 +- .../directive_processor/visitors.dart | 34 +++++- .../template_compiler/generator.dart | 4 +- .../common/async_string_writer_tests.dart | 106 ++++++++++++++++++ .../test/transform/common/read_file.dart | 20 +++- .../directive_processor/all_tests.dart | 15 ++- .../expected/hello.ng_deps.dart | 20 ++++ .../url_expression_files/hello.dart | 8 ++ .../url_expression_files/template.html | 1 + .../test/transform/transform.server.spec.dart | 2 + 13 files changed, 294 insertions(+), 24 deletions(-) create mode 100644 modules/angular2/src/transform/common/async_string_writer.dart rename modules/angular2/src/transform/{template_compiler => common}/xhr_impl.dart (100%) create mode 100644 modules/angular2/test/transform/common/async_string_writer_tests.dart create mode 100644 modules/angular2/test/transform/directive_processor/url_expression_files/expected/hello.ng_deps.dart create mode 100644 modules/angular2/test/transform/directive_processor/url_expression_files/hello.dart create mode 100644 modules/angular2/test/transform/directive_processor/url_expression_files/template.html diff --git a/modules/angular2/src/transform/common/async_string_writer.dart b/modules/angular2/src/transform/common/async_string_writer.dart new file mode 100644 index 0000000000..14d562de55 --- /dev/null +++ b/modules/angular2/src/transform/common/async_string_writer.dart @@ -0,0 +1,75 @@ +library angular2.transform.common.async_string_writer; + +import 'dart:async'; +import 'package:analyzer/src/generated/java_core.dart'; + +/// [PrintWriter] implementation that allows asynchronous printing via +/// [asyncPrint] and [asyncToString]. See those methods for details. +class AsyncStringWriter extends PrintWriter { + /// All [Future]s we are currently waiting on. + final List> _toAwait = >[]; + final List _bufs; + StringBuffer _curr; + int _asyncCount = 0; + + AsyncStringWriter._(StringBuffer curr) + : _curr = curr, + _bufs = [curr]; + + AsyncStringWriter() : this._(new StringBuffer()); + + void print(x) { + _curr.write(x); + } + + /// Adds the result of `futureText` to the writer at the current position + /// in the string being built. If using this method, you must use + /// [asyncToString] instead of [toString] to get the value of the writer or + /// your string may not appear as expected. + Future asyncPrint(Future futureText) { + _semaphoreIncrement(); + var myBuf = new StringBuffer(); + _bufs.add(myBuf); + _curr = new StringBuffer(); + _bufs.add(_curr); + + var toAwait = futureText.then((val) { + myBuf.write(val); + return val; + }); + _toAwait.add(toAwait); + return toAwait.whenComplete(() { + _semaphoreDecrementAndCleanup(); + _toAwait.remove(toAwait); + }); + } + + /// Waits for any values added via [asyncPrint] and returns the fully + /// built string. + Future asyncToString() { + _semaphoreIncrement(); + var bufLen = _bufs.length; + return Future.wait(_toAwait).then((_) { + return _bufs.sublist(0, bufLen).join(''); + }).whenComplete(_semaphoreDecrementAndCleanup); + } + + String toString() => _bufs.map((buf) => '$buf').join('(async gap)'); + + void _semaphoreIncrement() { + ++_asyncCount; + } + + void _semaphoreDecrementAndCleanup() { + assert(_asyncCount > 0); + + --_asyncCount; + if (_asyncCount == 0) { + _curr = _bufs[0]; + for (var i = 1; i < _bufs.length; ++i) { + _curr.write('${_bufs[i]}'); + } + _bufs.removeRange(1, _bufs.length); + } + } +} diff --git a/modules/angular2/src/transform/template_compiler/xhr_impl.dart b/modules/angular2/src/transform/common/xhr_impl.dart similarity index 100% rename from modules/angular2/src/transform/template_compiler/xhr_impl.dart rename to modules/angular2/src/transform/common/xhr_impl.dart diff --git a/modules/angular2/src/transform/directive_processor/rewriter.dart b/modules/angular2/src/transform/directive_processor/rewriter.dart index 8e9bc4cd76..142db35bee 100644 --- a/modules/angular2/src/transform/directive_processor/rewriter.dart +++ b/modules/angular2/src/transform/directive_processor/rewriter.dart @@ -1,10 +1,15 @@ library angular2.transform.directive_processor.rewriter; +import 'dart:async'; + import 'package:analyzer/analyzer.dart'; -import 'package:analyzer/src/generated/java_core.dart'; +import 'package:angular2/src/services/xhr.dart' show XHR; import 'package:angular2/src/transform/common/annotation_matcher.dart'; +import 'package:angular2/src/transform/common/asset_reader.dart'; +import 'package:angular2/src/transform/common/async_string_writer.dart'; import 'package:angular2/src/transform/common/logging.dart'; import 'package:angular2/src/transform/common/names.dart'; +import 'package:angular2/src/transform/common/xhr_impl.dart'; import 'package:barback/barback.dart' show AssetId; import 'package:path/path.dart' as path; @@ -17,20 +22,22 @@ import 'visitors.dart'; /// If no Angular 2 `Directive`s are found in `code`, returns the empty /// string unless `forceGenerate` is true, in which case an empty ngDeps /// file is created. -String createNgDeps( - String code, AssetId assetId, AnnotationMatcher annotationMatcher) { +Future createNgDeps(AssetReader reader, AssetId assetId, + AnnotationMatcher annotationMatcher) async { // TODO(kegluneq): Shortcut if we can determine that there are no // [Directive]s present, taking into account `export`s. - var writer = new PrintStringWriter(); - var visitor = new CreateNgDepsVisitor(writer, assetId, annotationMatcher); - parseCompilationUnit(code, name: assetId.toString()).accept(visitor); - return '$writer'; + var writer = new AsyncStringWriter(); + var visitor = new CreateNgDepsVisitor( + writer, assetId, new XhrImpl(reader, assetId), annotationMatcher); + var code = await reader.readAsString(assetId); + parseCompilationUnit(code, name: assetId.path).accept(visitor); + return await writer.asyncToString(); } /// Visitor responsible for processing [CompilationUnit] and creating an /// associated .ng_deps.dart file. class CreateNgDepsVisitor extends Object with SimpleAstVisitor { - final PrintWriter writer; + final AsyncStringWriter writer; bool _foundNgDirectives = false; bool _wroteImport = false; final ToSourceVisitor _copyVisitor; @@ -42,12 +49,13 @@ class CreateNgDepsVisitor extends Object with SimpleAstVisitor { /// The assetId for the file which we are parsing. final AssetId assetId; - CreateNgDepsVisitor(PrintWriter writer, this.assetId, this._annotationMatcher) + CreateNgDepsVisitor( + AsyncStringWriter writer, this.assetId, XHR xhr, this._annotationMatcher) : writer = writer, _copyVisitor = new ToSourceVisitor(writer), _factoryVisitor = new FactoryTransformVisitor(writer), _paramsVisitor = new ParameterTransformVisitor(writer), - _metaVisitor = new AnnotationsTransformVisitor(writer); + _metaVisitor = new AnnotationsTransformVisitor(writer, xhr); void _visitNodeListWithSeparator(NodeList list, String separator) { if (list == null) return; diff --git a/modules/angular2/src/transform/directive_processor/transformer.dart b/modules/angular2/src/transform/directive_processor/transformer.dart index 5d3bc6bb7a..211800a190 100644 --- a/modules/angular2/src/transform/directive_processor/transformer.dart +++ b/modules/angular2/src/transform/directive_processor/transformer.dart @@ -2,6 +2,7 @@ library angular2.transform.directive_processor.transformer; import 'dart:async'; +import 'package:angular2/src/transform/common/asset_reader.dart'; import 'package:angular2/src/transform/common/logging.dart' as log; import 'package:angular2/src/transform/common/names.dart'; import 'package:angular2/src/transform/common/options.dart'; @@ -32,9 +33,9 @@ class DirectiveProcessor extends Transformer { try { var asset = transform.primaryInput; - var assetCode = await asset.readAsString(); + var reader = new AssetReader.fromTransform(transform); var ngDepsSrc = - createNgDeps(assetCode, asset.id, options.annotationMatcher); + await createNgDeps(reader, asset.id, options.annotationMatcher); if (ngDepsSrc != null && ngDepsSrc.isNotEmpty) { var ngDepsAssetId = transform.primaryInput.id.changeExtension(DEPS_EXTENSION); diff --git a/modules/angular2/src/transform/directive_processor/visitors.dart b/modules/angular2/src/transform/directive_processor/visitors.dart index 2f307976ec..b876027b8e 100644 --- a/modules/angular2/src/transform/directive_processor/visitors.dart +++ b/modules/angular2/src/transform/directive_processor/visitors.dart @@ -2,6 +2,8 @@ library angular2.transform.directive_processor.visitors; import 'package:analyzer/analyzer.dart'; import 'package:analyzer/src/generated/java_core.dart'; +import 'package:angular2/src/services/xhr.dart' show XHR; +import 'package:angular2/src/transform/common/async_string_writer.dart'; import 'package:angular2/src/transform/common/logging.dart'; /// `ToSourceVisitor` designed to accept {@link ConstructorDeclaration} nodes. @@ -200,11 +202,17 @@ class FactoryTransformVisitor extends _CtorTransformVisitor { } } +// TODO(kegluenq): Use pull #1772 to detect when available. +bool _isViewAnnotation(Annotation node) => '${node.name}' == 'View'; + /// ToSourceVisitor designed to print a `ClassDeclaration` node as a /// 'annotations' value for Angular2's `registerType` calls. class AnnotationsTransformVisitor extends ToSourceVisitor { - final PrintWriter writer; - AnnotationsTransformVisitor(PrintWriter writer) + final AsyncStringWriter writer; + final XHR _xhr; + bool _processingView = false; + + AnnotationsTransformVisitor(AsyncStringWriter writer, this._xhr) : this.writer = writer, super(writer); @@ -226,6 +234,7 @@ class AnnotationsTransformVisitor extends ToSourceVisitor { Object visitAnnotation(Annotation node) { writer.print('const '); if (node.name != null) { + _processingView = _isViewAnnotation(node); node.name.accept(this); } if (node.constructorName != null) { @@ -237,4 +246,25 @@ class AnnotationsTransformVisitor extends ToSourceVisitor { } return null; } + + /// These correspond to the annotation parameters. + @override + Object visitNamedExpression(NamedExpression node) { + // TODO(kegluneq): Remove this limitation. + if (!_processingView || + node.name is! Label || + node.name.label is! SimpleIdentifier) { + return super.visitNamedExpression(node); + } + var keyString = '${node.name.label}'; + if (keyString == 'templateUrl' && node.expression is SimpleStringLiteral) { + var url = stringLiteralToString(node.expression); + writer.print("template: r'''"); + writer.asyncPrint(_xhr.get(url)); + writer.print("'''"); + return null; + } else { + return super.visitNamedExpression(node); + } + } } diff --git a/modules/angular2/src/transform/template_compiler/generator.dart b/modules/angular2/src/transform/template_compiler/generator.dart index 52ff5a3eee..da9e1b80e9 100644 --- a/modules/angular2/src/transform/template_compiler/generator.dart +++ b/modules/angular2/src/transform/template_compiler/generator.dart @@ -7,18 +7,18 @@ import 'package:angular2/src/change_detection/parser/parser.dart' as ng; import 'package:angular2/src/render/api.dart'; import 'package:angular2/src/render/dom/compiler/compiler.dart'; import 'package:angular2/src/render/dom/compiler/template_loader.dart'; -import "package:angular2/src/services/xhr.dart" show XHR; +import 'package:angular2/src/services/xhr.dart' show XHR; import 'package:angular2/src/reflection/reflection.dart'; import 'package:angular2/src/services/url_resolver.dart'; import 'package:angular2/src/transform/common/asset_reader.dart'; import 'package:angular2/src/transform/common/names.dart'; import 'package:angular2/src/transform/common/property_utils.dart' as prop; +import 'package:angular2/src/transform/common/xhr_impl.dart'; import 'package:barback/barback.dart'; import 'compile_step_factory.dart'; import 'recording_reflection_capabilities.dart'; import 'view_definition_creator.dart'; -import 'xhr_impl.dart'; /// Reads the `.ng_deps.dart` file represented by `entryPoint` and parses any /// Angular 2 `View` annotations it declares to generate `getter`s, diff --git a/modules/angular2/test/transform/common/async_string_writer_tests.dart b/modules/angular2/test/transform/common/async_string_writer_tests.dart new file mode 100644 index 0000000000..933219b195 --- /dev/null +++ b/modules/angular2/test/transform/common/async_string_writer_tests.dart @@ -0,0 +1,106 @@ +library angular2.test.transform.common.async_string_writer; + +import 'dart:async'; +import 'package:angular2/src/transform/common/async_string_writer.dart'; +import 'package:guinness/guinness.dart'; + +void allTests() { + it('should function as a basic Writer without async calls.', () { + var writer = new AsyncStringWriter(); + writer.print('hello'); + expect('$writer').toEqual('hello'); + writer.println(', world'); + expect('$writer').toEqual('hello, world\n'); + }); + + it('should concatenate futures added with `asyncPrint`.', () async { + var writer = new AsyncStringWriter(); + writer.print('hello'); + expect('$writer').toEqual('hello'); + writer.asyncPrint(new Future.value(', world.')); + writer.print(' It is a beautiful day.'); + expect(await writer.asyncToString()) + .toEqual('hello, world. It is a beautiful day.'); + }); + + it('should concatenate multiple futures regardless of order.', () async { + var completer1 = new Completer(); + var completer2 = new Completer(); + + var writer = new AsyncStringWriter(); + writer.print('hello'); + expect('$writer').toEqual('hello'); + writer.asyncPrint(completer1.future); + writer.asyncPrint(completer2.future); + + completer2.complete(' It is a beautiful day.'); + completer1.complete(', world.'); + + expect(await writer.asyncToString()) + .toEqual('hello, world. It is a beautiful day.'); + }); + + it('should allow multiple "rounds" of `asyncPrint`.', () async { + var writer = new AsyncStringWriter(); + writer.print('hello'); + expect('$writer').toEqual('hello'); + writer.asyncPrint(new Future.value(', world.')); + expect(await writer.asyncToString()).toEqual('hello, world.'); + + writer.asyncPrint(new Future.value(' It is ')); + writer.asyncPrint(new Future.value('a beautiful ')); + writer.asyncPrint(new Future.value('day.')); + + expect(await writer.asyncToString()) + .toEqual('hello, world. It is a beautiful day.'); + }); + + it('should handle calls to async methods while waiting.', () { + var completer1 = new Completer(); + var completer2 = new Completer(); + + var writer = new AsyncStringWriter(); + writer.print('hello'); + expect('$writer').toEqual('hello'); + + writer.asyncPrint(completer1.future); + var f1 = writer.asyncToString().then((result) { + expect(result).toEqual('hello, world.'); + }); + + writer.asyncPrint(completer2.future); + var f2 = writer.asyncToString().then((result) { + expect(result).toEqual('hello, world. It is a beautiful day.'); + }); + + completer1.complete(', world.'); + completer2.complete(' It is a beautiful day.'); + + return Future.wait([f1, f2]); + }); + + it('should handle calls to async methods that complete in reverse ' + 'order while waiting.', () { + var completer1 = new Completer(); + var completer2 = new Completer(); + + var writer = new AsyncStringWriter(); + writer.print('hello'); + expect('$writer').toEqual('hello'); + + writer.asyncPrint(completer1.future); + var f1 = writer.asyncToString().then((result) { + expect(result).toEqual('hello, world.'); + }); + + writer.asyncPrint(completer2.future); + var f2 = writer.asyncToString().then((result) { + expect(result).toEqual('hello, world. It is a beautiful day.'); + }); + + completer2.complete(' It is a beautiful day.'); + completer1.complete(', world.'); + + return Future.wait([f1, f2]); + }); +} diff --git a/modules/angular2/test/transform/common/read_file.dart b/modules/angular2/test/transform/common/read_file.dart index 53ae3ded61..257835881a 100644 --- a/modules/angular2/test/transform/common/read_file.dart +++ b/modules/angular2/test/transform/common/read_file.dart @@ -19,15 +19,29 @@ String readFile(String path) { } class TestAssetReader implements AssetReader { - Future readAsString(AssetId id, {Encoding encoding}) => - new Future.value(readFile(id.path)); + /// This allows "faking" + final Map _overrideAssets = {}; + + Future readAsString(AssetId id, {Encoding encoding}) { + if (_overrideAssets.containsKey(id)) { + return new Future.value(_overrideAssets[id]); + } else { + return new Future.value(readFile(id.path)); + } + } Future hasInput(AssetId id) { - var exists = false; + var exists = _overrideAssets.containsKey(id); + if (exists) return new Future.value(true); + for (var myPath in [id.path, 'test/transform/${id.path}']) { var file = new File(myPath); exists = exists || file.existsSync(); } return new Future.value(exists); } + + void addAsset(AssetId id, String contents) { + _overrideAssets[id] = contents; + } } diff --git a/modules/angular2/test/transform/directive_processor/all_tests.dart b/modules/angular2/test/transform/directive_processor/all_tests.dart index 70b4f29431..3a7cfbe983 100644 --- a/modules/angular2/test/transform/directive_processor/all_tests.dart +++ b/modules/angular2/test/transform/directive_processor/all_tests.dart @@ -3,10 +3,10 @@ library angular2.test.transform.directive_processor.all_tests; import 'package:barback/barback.dart'; import 'package:angular2/src/transform/directive_processor/rewriter.dart'; import 'package:angular2/src/transform/common/annotation_matcher.dart'; -import '../common/read_file.dart'; import 'package:dart_style/dart_style.dart'; import 'package:guinness/guinness.dart'; import 'package:path/path.dart' as path; +import '../common/read_file.dart'; var formatter = new DartFormatter(); @@ -36,6 +36,9 @@ void allTests() { customDescriptors: [ const AnnotationDescriptor('Soup', 'package:soup/soup.dart', 'Component'), ]); + + _testNgDeps( + 'should inline `templateUrl` values.', 'url_expression_files/hello.dart'); } void _testNgDeps(String name, String inputPath, @@ -43,11 +46,13 @@ void _testNgDeps(String name, String inputPath, it(name, () async { var inputId = _assetIdForPath(inputPath); var reader = new TestAssetReader(); - var input = await reader.readAsString(inputId); + if (assetId != null) { + reader.addAsset(assetId, await reader.readAsString(inputId)); + inputId = assetId; + } var annotationMatcher = new AnnotationMatcher()..addAll(customDescriptors); - var proxyInputId = assetId != null ? assetId : inputId; - var output = - formatter.format(createNgDeps(input, proxyInputId, annotationMatcher)); + var output = formatter + .format(await createNgDeps(reader, inputId, annotationMatcher)); var expectedPath = path.join(path.dirname(inputPath), 'expected', path.basename(inputPath).replaceFirst('.dart', '.ng_deps.dart')); var expectedId = _assetIdForPath(expectedPath); diff --git a/modules/angular2/test/transform/directive_processor/url_expression_files/expected/hello.ng_deps.dart b/modules/angular2/test/transform/directive_processor/url_expression_files/expected/hello.ng_deps.dart new file mode 100644 index 0000000000..b877935fb0 --- /dev/null +++ b/modules/angular2/test/transform/directive_processor/url_expression_files/expected/hello.ng_deps.dart @@ -0,0 +1,20 @@ +library examples.src.hello_world.index_common_dart.ng_deps.dart; + +import 'hello.dart'; +import 'package:angular2/angular2.dart' + show bootstrap, Component, Directive, View, NgElement; + +var _visited = false; +void initReflector(reflector) { + if (_visited) return; + _visited = true; + reflector + ..registerType(HelloCmp, { + 'factory': () => new HelloCmp(), + 'parameters': const [], + 'annotations': const [ + const Component(selector: 'hello-app'), + const View(template: r'''{{greeting}}''') + ] + }); +} diff --git a/modules/angular2/test/transform/directive_processor/url_expression_files/hello.dart b/modules/angular2/test/transform/directive_processor/url_expression_files/hello.dart new file mode 100644 index 0000000000..5471f1eb1b --- /dev/null +++ b/modules/angular2/test/transform/directive_processor/url_expression_files/hello.dart @@ -0,0 +1,8 @@ +library examples.src.hello_world.index_common_dart; + +import 'package:angular2/angular2.dart' + show bootstrap, Component, Directive, View, NgElement; + +@Component(selector: 'hello-app') +@View(templateUrl: 'template.html') +class HelloCmp {} diff --git a/modules/angular2/test/transform/directive_processor/url_expression_files/template.html b/modules/angular2/test/transform/directive_processor/url_expression_files/template.html new file mode 100644 index 0000000000..d75013393f --- /dev/null +++ b/modules/angular2/test/transform/directive_processor/url_expression_files/template.html @@ -0,0 +1 @@ +{{greeting}} \ No newline at end of file diff --git a/modules/angular2/test/transform/transform.server.spec.dart b/modules/angular2/test/transform/transform.server.spec.dart index 38ab159f1f..8ed7b90e37 100644 --- a/modules/angular2/test/transform/transform.server.spec.dart +++ b/modules/angular2/test/transform/transform.server.spec.dart @@ -4,6 +4,7 @@ import 'package:guinness/guinness.dart'; import 'package:unittest/unittest.dart' hide expect; import 'package:unittest/vm_config.dart'; +import 'common/async_string_writer_tests.dart' as asyncStringWriter; import 'bind_generator/all_tests.dart' as bindGenerator; import 'directive_linker/all_tests.dart' as directiveLinker; import 'directive_metadata_extractor/all_tests.dart' as directiveMeta; @@ -14,6 +15,7 @@ import 'template_compiler/all_tests.dart' as templateCompiler; main() { useVMConfiguration(); + describe('AsyncStringWriter', asyncStringWriter.allTests); describe('Bind Generator', bindGenerator.allTests); describe('Directive Linker', directiveLinker.allTests); describe('Directive Metadata Extractor', directiveMeta.allTests);