From 1f2302e39e42220e0ad5179fb63abbd667e3d1f3 Mon Sep 17 00:00:00 2001 From: Tim Blasi Date: Mon, 21 Sep 2015 16:00:52 -0700 Subject: [PATCH] fix(dart/transform): Fix transformer output declaration Ensure that the transformers are properly declaring all consumed outputs. In particular, when a transformer overwrites an output, make sure the transformer calls `consumePrimary` followed by `addOutput` for that file. Prevent `DirectiveLinker` from consuming `.ng_deps.json` files. When we do this, barback incorrectly calculates the available assets for that phase, resulting in broken builds. --- .../transform/bind_generator/transformer.dart | 8 +++-- .../deferred_rewriter/transformer.dart | 7 ++-- .../directive_linker/transformer.dart | 35 +++++++++++++------ .../directive_processor/transformer.dart | 29 +++++++-------- .../reflection_remover/transformer.dart | 8 +++-- .../template_compiler/transformer.dart | 9 +++-- 6 files changed, 58 insertions(+), 38 deletions(-) diff --git a/modules_dart/transform/lib/src/transform/bind_generator/transformer.dart b/modules_dart/transform/lib/src/transform/bind_generator/transformer.dart index e5e5999a1f..75e79870d4 100644 --- a/modules_dart/transform/lib/src/transform/bind_generator/transformer.dart +++ b/modules_dart/transform/lib/src/transform/bind_generator/transformer.dart @@ -26,17 +26,19 @@ class BindGenerator extends Transformer implements DeclaringTransformer { @override declareOutputs(DeclaringTransform transform) { + transform.consumePrimary(); transform.declareOutput(transform.primaryId); } @override Future apply(Transform transform) async { await log.initZoned(transform, () async { - var id = transform.primaryInput.id; + var primaryId = transform.primaryInput.id; var reader = new AssetReader.fromTransform(transform); - var transformedCode = await createNgSettersAndGetters(reader, id); + var transformedCode = await createNgSettersAndGetters(reader, primaryId); + transform.consumePrimary(); transform.addOutput(new Asset.fromString( - id, formatter.format(transformedCode, uri: id.path))); + primaryId, formatter.format(transformedCode, uri: primaryId.path))); }); } } diff --git a/modules_dart/transform/lib/src/transform/deferred_rewriter/transformer.dart b/modules_dart/transform/lib/src/transform/deferred_rewriter/transformer.dart index df71b80ea8..5350948660 100644 --- a/modules_dart/transform/lib/src/transform/deferred_rewriter/transformer.dart +++ b/modules_dart/transform/lib/src/transform/deferred_rewriter/transformer.dart @@ -24,6 +24,7 @@ class DeferredRewriter extends Transformer implements DeclaringTransformer { @override declareOutputs(DeclaringTransform transform) { + transform.consumePrimary(); transform.declareOutput(transform.primaryId); } @@ -33,9 +34,11 @@ class DeferredRewriter extends Transformer implements DeclaringTransformer { var asset = transform.primaryInput; var reader = new AssetReader.fromTransform(transform); var transformedCode = await rewriteDeferredLibraries(reader, asset.id); + transform.consumePrimary(); if (transformedCode != null) { - transform.addOutput( - new Asset.fromString(transform.primaryInput.id, transformedCode)); + transform.addOutput(new Asset.fromString(asset.id, transformedCode)); + } else { + transform.addOutput(asset); } }); } diff --git a/modules_dart/transform/lib/src/transform/directive_linker/transformer.dart b/modules_dart/transform/lib/src/transform/directive_linker/transformer.dart index 37a53ee366..eed6d14bf8 100644 --- a/modules_dart/transform/lib/src/transform/directive_linker/transformer.dart +++ b/modules_dart/transform/lib/src/transform/directive_linker/transformer.dart @@ -22,7 +22,10 @@ class DirectiveLinker extends Transformer implements DeclaringTransformer { @override declareOutputs(DeclaringTransform transform) { - transform.declareOutput(transform.primaryId); + // TODO(kegluenq): We should consume this, but doing so causes barback to + // incorrectly determine what assets are available in this phase. + // transform.consumePrimary(); + transform.declareOutput(_depsAssetId(transform.primaryId)); } @override @@ -30,21 +33,26 @@ class DirectiveLinker extends Transformer implements DeclaringTransformer { await log.initZoned(transform, () async { var reader = new AssetReader.fromTransform(transform); var assetId = transform.primaryInput.id; - var assetPath = assetId.path; var ngDepsModel = await linkNgDeps(reader, assetId); + // See above + // transform.consumePrimary(); + var outputAssetId = _depsAssetId(assetId); if (ngDepsModel != null) { var buf = new StringBuffer(); var writer = new NgDepsWriter(buf); writer.writeNgDepsModel(ngDepsModel); - var formattedCode = formatter.format('$buf', uri: assetPath); - var ngDepsAssetId = - new AssetId(assetId.package, toDepsExtension(assetPath)); - transform.addOutput(new Asset.fromString(ngDepsAssetId, formattedCode)); + var formattedCode = formatter.format('$buf', uri: assetId.path); + transform.addOutput(new Asset.fromString(outputAssetId, formattedCode)); + } else { + transform.addOutput(new Asset.fromString(outputAssetId, '')); } }); } } +AssetId _depsAssetId(AssetId primaryId) => + new AssetId(primaryId.package, toDepsExtension(primaryId.path)); + /// Transformer responsible for removing unnecessary `.ng_deps.json` files /// created by {@link DirectiveProcessor}. class EmptyNgDepsRemover extends Transformer implements DeclaringTransformer { @@ -54,17 +62,22 @@ class EmptyNgDepsRemover extends Transformer implements DeclaringTransformer { bool isPrimary(AssetId id) => id.path.endsWith(DEPS_JSON_EXTENSION); /// We occasionally consume the primary input, but that depends on the - /// contents of the file, so we conservatively do not declare any outputs nor - /// consumption to ensure that we declare a superset of our actual outputs. + /// contents of the file, so we conservatively declare that we both consume + /// and output the asset. This prevents barback from making any assumptions + /// about the existence of the assets until after the transformer has run. @override - declareOutputs(DeclaringTransform transform) => null; + declareOutputs(DeclaringTransform transform) { + transform.consumePrimary(); + transform.declareOutput(transform.primaryId); + } @override Future apply(Transform transform) async { await log.initZoned(transform, () async { var reader = new AssetReader.fromTransform(transform); - if (!(await isNecessary(reader, transform.primaryInput.id))) { - transform.consumePrimary(); + transform.consumePrimary(); + if ((await isNecessary(reader, transform.primaryInput.id))) { + transform.addOutput(transform.primaryInput); } }); } diff --git a/modules_dart/transform/lib/src/transform/directive_processor/transformer.dart b/modules_dart/transform/lib/src/transform/directive_processor/transformer.dart index 6ec3e953cd..c7f6abba09 100644 --- a/modules_dart/transform/lib/src/transform/directive_processor/transformer.dart +++ b/modules_dart/transform/lib/src/transform/directive_processor/transformer.dart @@ -34,36 +34,33 @@ class DirectiveProcessor extends Transformer implements DeclaringTransformer { /// determine that one or the other will not be emitted. @override declareOutputs(DeclaringTransform transform) { - transform.declareOutput( - transform.primaryId.changeExtension(ALIAS_EXTENSION)); - transform.declareOutput( - transform.primaryId.changeExtension(DEPS_EXTENSION)); + transform.declareOutput(_depsOutputId(transform.primaryId)); + transform.declareOutput(_metaOutputId(transform.primaryId)); } @override Future apply(Transform transform) async { await log.initZoned(transform, () async { - var asset = transform.primaryInput; + var assetId = transform.primaryInput.id; var reader = new AssetReader.fromTransform(transform); var ngMeta = new NgMeta.empty(); var ngDepsModel = await createNgDeps( - reader, asset.id, options.annotationMatcher, ngMeta, + reader, assetId, options.annotationMatcher, ngMeta, inlineViews: options.inlineViews); if (ngDepsModel != null) { - var ngDepsAssetId = - transform.primaryInput.id.changeExtension(DEPS_JSON_EXTENSION); - if (await transform.hasInput(ngDepsAssetId)) { - log.logger.error('Clobbering ${ngDepsAssetId}. ' - 'This probably will not end well'); - } - transform.addOutput(new Asset.fromString(ngDepsAssetId, ngDepsModel.writeToJson())); + transform.addOutput(new Asset.fromString( + _depsOutputId(assetId), ngDepsModel.writeToJson())); } + var metaOutputId = _metaOutputId(assetId); if (!ngMeta.isEmpty) { - var ngAliasesId = - transform.primaryInput.id.changeExtension(ALIAS_EXTENSION); - transform.addOutput(new Asset.fromString(ngAliasesId, + transform.addOutput(new Asset.fromString(metaOutputId, new JsonEncoder.withIndent(" ").convert(ngMeta.toJson()))); } }); } } + +AssetId _depsOutputId(AssetId primaryId) => + primaryId.changeExtension(DEPS_JSON_EXTENSION); +AssetId _metaOutputId(AssetId primaryId) => + primaryId.changeExtension(ALIAS_EXTENSION); diff --git a/modules_dart/transform/lib/src/transform/reflection_remover/transformer.dart b/modules_dart/transform/lib/src/transform/reflection_remover/transformer.dart index 6a37503bc3..de0fe8a672 100644 --- a/modules_dart/transform/lib/src/transform/reflection_remover/transformer.dart +++ b/modules_dart/transform/lib/src/transform/reflection_remover/transformer.dart @@ -30,12 +30,14 @@ class ReflectionRemover extends Transformer implements DeclaringTransformer { @override declareOutputs(DeclaringTransform transform) { + transform.consumePrimary(); transform.declareOutput(transform.primaryId); } @override Future apply(Transform transform) async { await log.initZoned(transform, () async { + var primaryId = transform.primaryInput.id; var mirrorMode = options.mirrorMode; var writeStaticInit = options.initReflector; if (options.modeName == TRANSFORM_DYNAMIC_MODE) { @@ -47,10 +49,10 @@ class ReflectionRemover extends Transformer implements DeclaringTransformer { } var transformedCode = await removeReflectionCapabilities( - new AssetReader.fromTransform(transform), transform.primaryInput.id, + new AssetReader.fromTransform(transform), primaryId, mirrorMode: mirrorMode, writeStaticInit: writeStaticInit); - transform.addOutput( - new Asset.fromString(transform.primaryInput.id, transformedCode)); + transform.consumePrimary(); + transform.addOutput(new Asset.fromString(primaryId, transformedCode)); }); } } diff --git a/modules_dart/transform/lib/src/transform/template_compiler/transformer.dart b/modules_dart/transform/lib/src/transform/template_compiler/transformer.dart index aa7be1313f..e938cb8aab 100644 --- a/modules_dart/transform/lib/src/transform/template_compiler/transformer.dart +++ b/modules_dart/transform/lib/src/transform/template_compiler/transformer.dart @@ -28,6 +28,7 @@ class TemplateCompiler extends Transformer implements DeclaringTransformer { @override declareOutputs(DeclaringTransform transform) { + transform.consumePrimary(); transform.declareOutput(transform.primaryId); } @@ -35,13 +36,15 @@ class TemplateCompiler extends Transformer implements DeclaringTransformer { Future apply(Transform transform) async { await log.initZoned(transform, () async { Html5LibDomAdapter.makeCurrent(); - var id = transform.primaryInput.id; + var primaryId = transform.primaryInput.id; var reader = new AssetReader.fromTransform(transform); - var transformedCode = formatter.format(await processTemplates(reader, id, + var transformedCode = formatter.format(await processTemplates( + reader, primaryId, generateChangeDetectors: options.generateChangeDetectors, reflectPropertiesAsAttributes: options.reflectPropertiesAsAttributes)); - transform.addOutput(new Asset.fromString(id, transformedCode)); + transform.consumePrimary(); + transform.addOutput(new Asset.fromString(primaryId, transformedCode)); }); } }