From 9d0b61bad5fd50c84fac541c30f9f7e153fb9ce0 Mon Sep 17 00:00:00 2001 From: Tim Blasi Date: Thu, 5 Nov 2015 11:27:21 -0800 Subject: [PATCH] feat(dart/transform): Simplify dependency imports Store dependency import information in a dedicated list in `NgDepsModel` rather than as a boolean field on `ImportModel`. An `ImportModel` should not "care" whether it is a .ng_deps.dart import or not -- this information belongs in `NgDepsModel`. This simplifies some of the logic around how `NgDepsModel` imports are processed and eventually output. --- .../transform/common/code/ng_deps_code.dart | 9 +++-- .../common/model/import_export_model.pb.dart | 14 ++------ .../common/model/import_export_model.proto | 2 -- .../common/model/ng_deps_model.pb.dart | 15 ++++++-- .../common/model/ng_deps_model.proto | 4 +++ .../ng_deps_linker.dart | 36 +++++++------------ .../compile_data_creator.dart | 24 ++++++------- 7 files changed, 47 insertions(+), 57 deletions(-) diff --git a/modules_dart/transform/lib/src/transform/common/code/ng_deps_code.dart b/modules_dart/transform/lib/src/transform/common/code/ng_deps_code.dart index 8d5c48fc68..b6b4db68e3 100644 --- a/modules_dart/transform/lib/src/transform/common/code/ng_deps_code.dart +++ b/modules_dart/transform/lib/src/transform/common/code/ng_deps_code.dart @@ -157,6 +157,7 @@ abstract class NgDepsWriterMixin // deferred. Instead `DeferredRewriter` will rewrite the code as to load // `ng_deps` in a deferred way. model.imports.where((i) => !i.isDeferred).forEach(writeImportModel); + model.depImports.where((i) => !i.isDeferred).forEach(writeImportModel); writeExportModel(new ExportModel()..uri = model.sourceFile); model.exports.forEach(writeExportModel); @@ -202,11 +203,9 @@ abstract class NgDepsWriterMixin buffer.writeln(';'); } - // Call the setup method for our imports that are `.ng_deps` imports. - for (var importModel in model.imports) { - if (importModel.isNgDeps) { - buffer.writeln('${importModel.prefix}.${SETUP_METHOD_NAME}();'); - } + // Call the setup method for our dependencies. + for (var importModel in model.depImports) { + buffer.writeln('${importModel.prefix}.${SETUP_METHOD_NAME}();'); } buffer.writeln('}'); diff --git a/modules_dart/transform/lib/src/transform/common/model/import_export_model.pb.dart b/modules_dart/transform/lib/src/transform/common/model/import_export_model.pb.dart index d8095a8bb2..a17811ff39 100644 --- a/modules_dart/transform/lib/src/transform/common/model/import_export_model.pb.dart +++ b/modules_dart/transform/lib/src/transform/common/model/import_export_model.pb.dart @@ -11,8 +11,7 @@ class ImportModel extends GeneratedMessage { ..p(2, 'showCombinators', PbFieldType.PS) ..p(3, 'hideCombinators', PbFieldType.PS) ..a(4, 'prefix', PbFieldType.OS) - ..a(5, 'isDeferred', PbFieldType.OB) - ..a(6, 'isNgDeps', PbFieldType.OB); + ..a(5, 'isDeferred', PbFieldType.OB); ImportModel() : super(); ImportModel.fromBuffer(List i, @@ -62,14 +61,6 @@ class ImportModel extends GeneratedMessage { bool hasIsDeferred() => $_has(4, 5); void clearIsDeferred() => clearField(5); - - bool get isNgDeps => $_get(5, 6, false); - void set isNgDeps(bool v) { - $_setBool(5, 6, v); - } - - bool hasIsNgDeps() => $_has(5, 6); - void clearIsNgDeps() => clearField(6); } class _ReadonlyImportModel extends ImportModel with ReadonlyMessageMixin {} @@ -124,7 +115,6 @@ const ImportModel$json = const { const {'1': 'hide_combinators', '3': 3, '4': 3, '5': 9}, const {'1': 'prefix', '3': 4, '4': 1, '5': 9}, const {'1': 'is_deferred', '3': 5, '4': 1, '5': 8}, - const {'1': 'is_ng_deps', '3': 6, '4': 1, '5': 8}, ], }; @@ -139,7 +129,7 @@ const ExportModel$json = const { /** * Generated with: - * import_export_model.proto (c018d2ad92db2d341631d813ace70925d1ccbb9b) + * import_export_model.proto (36a3a72d0884b84b451b7188ffa1fc93b44e7b62) * libprotoc 2.6.1 * dart-protoc-plugin (af5fc2bf1de367a434c3b1847ab260510878ffc0) */ diff --git a/modules_dart/transform/lib/src/transform/common/model/import_export_model.proto b/modules_dart/transform/lib/src/transform/common/model/import_export_model.proto index 0635276485..eb23a3b712 100644 --- a/modules_dart/transform/lib/src/transform/common/model/import_export_model.proto +++ b/modules_dart/transform/lib/src/transform/common/model/import_export_model.proto @@ -16,8 +16,6 @@ message ImportModel { optional string prefix = 4; optional bool is_deferred = 5; - - optional bool is_ng_deps = 6; } // See message above about wire-compatiblity with `ImportModel`. diff --git a/modules_dart/transform/lib/src/transform/common/model/ng_deps_model.pb.dart b/modules_dart/transform/lib/src/transform/common/model/ng_deps_model.pb.dart index 7ba9b86ef1..46f82d2a57 100644 --- a/modules_dart/transform/lib/src/transform/common/model/ng_deps_model.pb.dart +++ b/modules_dart/transform/lib/src/transform/common/model/ng_deps_model.pb.dart @@ -20,7 +20,9 @@ class NgDepsModel extends GeneratedMessage { ..a(6, 'sourceFile', PbFieldType.OS) ..p(7, 'getters', PbFieldType.PS) ..p(8, 'setters', PbFieldType.PS) - ..p(9, 'methods', PbFieldType.PS); + ..p(9, 'methods', PbFieldType.PS) + ..pp(10, 'depImports', PbFieldType.PM, ImportModel.$checkItem, + ImportModel.create); NgDepsModel() : super(); NgDepsModel.fromBuffer(List i, @@ -72,6 +74,8 @@ class NgDepsModel extends GeneratedMessage { List get setters => $_get(7, 8, null); List get methods => $_get(8, 9, null); + + List get depImports => $_get(9, 10, null); } class _ReadonlyNgDepsModel extends NgDepsModel with ReadonlyMessageMixin {} @@ -106,12 +110,19 @@ const NgDepsModel$json = const { const {'1': 'getters', '3': 7, '4': 3, '5': 9}, const {'1': 'setters', '3': 8, '4': 3, '5': 9}, const {'1': 'methods', '3': 9, '4': 3, '5': 9}, + const { + '1': 'dep_imports', + '3': 10, + '4': 3, + '5': 11, + '6': '.angular2.src.transform.common.model.proto.ImportModel' + }, ], }; /** * Generated with: - * ng_deps_model.proto (ab93a7f3c411be8a6dafe914f8aae56027e1bac6) + * ng_deps_model.proto (64702efcc1d7fb434f3652943ba051104960ffd5) * libprotoc 2.6.1 * dart-protoc-plugin (af5fc2bf1de367a434c3b1847ab260510878ffc0) */ diff --git a/modules_dart/transform/lib/src/transform/common/model/ng_deps_model.proto b/modules_dart/transform/lib/src/transform/common/model/ng_deps_model.proto index c408492dc3..5b48834bd4 100644 --- a/modules_dart/transform/lib/src/transform/common/model/ng_deps_model.proto +++ b/modules_dart/transform/lib/src/transform/common/model/ng_deps_model.proto @@ -32,4 +32,8 @@ message NgDepsModel { // The names of methods that should be registered in the Angular 2 reflection // framework. repeated string methods = 9; + + // Imports for the .ng_deps.dart files associated with the declared `imports` + // and `exports` for this file. + repeated ImportModel dep_imports = 10; } diff --git a/modules_dart/transform/lib/src/transform/directive_metadata_linker/ng_deps_linker.dart b/modules_dart/transform/lib/src/transform/directive_metadata_linker/ng_deps_linker.dart index f27bcd7425..88a8733d5f 100644 --- a/modules_dart/transform/lib/src/transform/directive_metadata_linker/ng_deps_linker.dart +++ b/modules_dart/transform/lib/src/transform/directive_metadata_linker/ng_deps_linker.dart @@ -15,10 +15,10 @@ import 'package:barback/barback.dart'; /// dependencies' associated `.ng_deps.dart` files. /// /// For example, if entry_point.ng_deps.dart imports dependency.dart, this -/// will check if dependency.ng_meta.json exists. If it does, we add an import -/// to dependency.ng_deps.dart to the entry_point [NgDepsModel] and set -/// `isNgDeps` to `true` to signify that it is a dependency on which we need to -/// call `initReflector`. +/// will check if dependency.ng_meta.json exists. If it does, we add an entry +/// to the `depImports` of [NgDepsModel] for dependency.ng_deps.dart. We can +/// use this information later to ensure that each file's dependencies are +/// initialized when that file is initialized. Future linkNgDeps(NgDepsModel ngDepsModel, AssetReader reader, AssetId assetId, UrlResolver resolver) async { if (ngDepsModel == null) return null; @@ -34,28 +34,16 @@ Future linkNgDeps(NgDepsModel ngDepsModel, AssetReader reader, } final seen = new Set(); - for (var i = ngDepsModel.imports.length - 1; i >= 0; --i) { - var import = ngDepsModel.imports[i]; - if (linkedDepsMap.containsKey(import.uri) && !seen.contains(import.uri)) { - seen.add(import.uri); + var idx = 0; + final allDeps = [ngDepsModel.imports, ngDepsModel.exports].expand((e) => e); + for (var dep in allDeps) { + if (linkedDepsMap.containsKey(dep.uri) && !seen.contains(dep.uri)) { + seen.add(dep.uri); var linkedModel = new ImportModel() - ..isNgDeps = true - ..uri = toDepsExtension(import.uri) - ..prefix = 'i$i'; + ..uri = toDepsExtension(dep.uri) + ..prefix = 'i${idx++}'; // TODO(kegluneq): Preserve combinators? - ngDepsModel.imports.insert(i + 1, linkedModel); - } - } - for (var i = 0, iLen = ngDepsModel.exports.length; i < iLen; ++i) { - var export = ngDepsModel.exports[i]; - if (linkedDepsMap.containsKey(export.uri) && !seen.contains(export.uri)) { - seen.add(export.uri); - var linkedModel = new ImportModel() - ..isNgDeps = true - ..uri = toDepsExtension(export.uri) - ..prefix = 'i${ngDepsModel.imports.length}'; - // TODO(kegluneq): Preserve combinators? - ngDepsModel.imports.add(linkedModel); + ngDepsModel.depImports.add(linkedModel); } } return ngDepsModel; diff --git a/modules_dart/transform/lib/src/transform/template_compiler/compile_data_creator.dart b/modules_dart/transform/lib/src/transform/template_compiler/compile_data_creator.dart index f0fca65b83..06eb09727c 100644 --- a/modules_dart/transform/lib/src/transform/template_compiler/compile_data_creator.dart +++ b/modules_dart/transform/lib/src/transform/template_compiler/compile_data_creator.dart @@ -22,7 +22,8 @@ import 'package:barback/barback.dart'; Future createCompileData( AssetReader reader, AssetId assetId, List ambientDirectives) async { return logElapsedAsync(() async { - final creator = await _CompileDataCreator.create(reader, assetId, ambientDirectives); + final creator = + await _CompileDataCreator.create(reader, assetId, ambientDirectives); return creator != null ? creator.createCompileData() : null; }, operationName: 'createCompileData', assetId: assetId); } @@ -43,10 +44,11 @@ class _CompileDataCreator { final NgMeta ngMeta; final List ambientDirectives; - _CompileDataCreator(this.reader, this.entryPoint, this.ngMeta, this.ambientDirectives); + _CompileDataCreator( + this.reader, this.entryPoint, this.ngMeta, this.ambientDirectives); - static Future<_CompileDataCreator> create( - AssetReader reader, AssetId assetId, List ambientDirectives) async { + static Future<_CompileDataCreator> create(AssetReader reader, AssetId assetId, + List ambientDirectives) async { if (!(await reader.hasInput(assetId))) return null; final json = await reader.readAsString(assetId); if (json == null || json.isEmpty) return null; @@ -119,7 +121,8 @@ class _CompileDataCreator { return res; } - Future> _readAmbientDirectivesFromUri(String uri, String token) async { + Future> _readAmbientDirectivesFromUri( + String uri, String token) async { final metaAssetId = fromUri(toMetaExtension(uri)); if (await reader.hasInput(metaAssetId)) { try { @@ -129,19 +132,16 @@ class _CompileDataCreator { if (newMetadata.types.containsKey(token)) { return [newMetadata.types[token]]; - } else if (newMetadata.aliases.containsKey(token)) { return newMetadata.flatten(token); - } else { - log.warning('Could not resolve ambient directive ${token} in ${uri}', + log.warning( + 'Could not resolve ambient directive ${token} in ${uri}', asset: metaAssetId); } - } } catch (ex, stackTrace) { - log.warning('Failed to decode: $ex, $stackTrace', - asset: metaAssetId); + log.warning('Failed to decode: $ex, $stackTrace', asset: metaAssetId); } } return []; @@ -160,7 +160,7 @@ class _CompileDataCreator { final resolver = const TransformerUrlResolver(); ngMeta.ngDeps.imports - .where((model) => !model.isNgDeps && !isDartCoreUri(model.uri)) + .where((model) => !isDartCoreUri(model.uri)) .forEach((model) { var prefix = model.prefix == null ? '' : model.prefix; map