Run declaring transformers eagerly if possible, even if their inputs are deferred.

pkg/barback/lib/src/transform_node.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library barback.transform_node;
 
 import 'dart:async';
 
 import 'asset.dart';
 import 'asset_id.dart';
@@ skipping 37 matching lines @@
   bool get isDirty => _state != _State.NOT_PRIMARY &&
       _state != _State.APPLIED && _state != _State.DECLARED;
 
   /// Whether this transform is deferred.
   ///
   /// A transform is deferred if either its transformer is lazy or if its
   /// transformer is declaring and its primary input comes from a deferred
   /// transformer.
   final bool deferred;
 
-  /// Whether this is a deferred transform waiting for [force] to be called to
-  /// generate inputs.
+  /// Whether this transform has been forced since it last finished applying.
   ///
-  /// This defaults to `true` for deferred transforms and `false` otherwise.
-  /// During or after running `isPrimary` or `declareOutputs`, this may become
-  /// `false`, indicating that the transform has been forced and should generate
-  /// outputs as soon as possible. It will only be set back to `true` if an
-  /// input changes *after* `apply` has completed.
-  bool _awaitingForce;
+  /// A transform being forced means it should run until it generates outputs
+  /// and is no longer dirty. This is always true for non-[deferred]
+  /// transformers, since they always need to eagerly generate outputs.
+  bool _forced;
 
   /// The subscriptions to each input's [AssetNode.onStateChange] stream.
   final _inputSubscriptions = new Map<AssetId, StreamSubscription>();
 
   /// The controllers for the asset nodes emitted by this node.
   final _outputControllers = new Map<AssetId, AssetNodeController>();
 
   /// The ids of inputs the transformer tried and failed to read last time it
   /// ran.
   final _missingInputs = new Set<AssetId>();
@@ skipping 51 matching lines @@
   /// This is only non-null if [transformer] is a [DeclaringTransformer] and its
   /// [declareOutputs] has been run successfully.
   Set<AssetId> _declaredOutputs;
 
   TransformNode(this.phase, Transformer transformer, AssetNode primary,
       this._location)
       : transformer = transformer,
         primary = primary,
         deferred = transformer is LazyTransformer ||
             (transformer is DeclaringTransformer && primary.deferred) {
-    _awaitingForce = deferred;
+    _forced = !deferred;
 
     _onLogPool.add(_onLogController.stream);
 
     _primarySubscription = primary.onStateChange.listen((state) {
       if (state.isRemoved) {
         remove();
       } else {
         if (state.isDirty && !deferred) primary.force();
         // If this is deferred but applying, that means it must have been
         // forced, so we should ensure its input remains forced as well.
-        if (deferred && _state == _State.APPLYING) primary.force();
+        if (deferred && _forced && _state == _State.APPLYING) primary.force();
         _dirty();
       }
     });
 
     _phaseSubscription = phase.previous.onAsset.listen((node) {
       if (!_missingInputs.contains(node.id)) return;
       if (!deferred) node.force();
       _dirty();
     });
 
@@ skipping 22 matching lines @@
     _clearOutputs();
     if (_passThroughController != null) {
       _passThroughController.setRemoved();
       _passThroughController = null;
     }
   }
 
   /// If [this] is deferred, ensures that its concrete outputs will be
   /// generated.
   void force() {
-    if (!_awaitingForce) return;
+    if (_forced || _state == _State.APPLIED) return;
     primary.force();
-    _awaitingForce = false;
+    _forced = true;
     _dirty();
   }
 
   /// Marks this transform as dirty.
   ///
   /// This causes all of the transform's outputs to be marked as dirty as well.
   void _dirty() {
     if (_state == _State.NOT_PRIMARY) {
       _emitPassThrough();
       return;
     }
 
     // If we're in the process of running [isPrimary] or [declareOutputs], we
     // already know that [apply] needs to be run so there's nothing we need to
     // mark as dirty.
     if (_state == _State.DECLARING) return;
 
-    // If we're waiting until [force] is called to run [apply], we don't want to
-    // run [apply] too early.
-    if (_awaitingForce) return;
+    if (!_forced &&
+        // If [transformer] is declaring and [primary] is available, we do want
+        // to start running [apply] even if [force] hasn't been called, since
+        // [transformer] should run eagerly if possible.
+        (transformer is LazyTransformer || !primary.state.isAvailable)) {

[Bob Nystrom, 2014/04/21 22:37:40]

This condition is pretty dense. How about breaking some of it out to a
well-named local variable?

I've read it several times, but I still don't get it. Why is it checking for "is
LazyTransformer" when the doc is about declaring ones?

[nweiz, 2014/04/21 22:52:09]

Done.
Because this is the branch where we *don't* run apply.

+      // [forced] should only ever be false for a deferred transform.
+      assert(deferred);
 
-    if (_state == _State.APPLIED && deferred) {
-      // Transition to DECLARED, indicating that we know what outputs [apply]
-      // will emit but we're waiting to emit them concretely until [force] is
-      // called.
-      _state = _State.DECLARED;
-      _awaitingForce = true;
+      // If we've finished applying, transition to DECLARED, indicating that we
+      // know what outputs [apply] will emit but we're waiting to emit them
+      // concretely until [force] is called. If we're still applying, we'll
+      // transition to DECLARED once we finish.
+      if (_state == _State.APPLIED) _state = _State.DECLARED;
       for (var controller in _outputControllers.values) {
         controller.setLazy(force);
       }
       return;
     }
 
     if (_passThroughController != null) _passThroughController.setDirty();
     for (var controller in _outputControllers.values) {
       controller.setDirty();
     }
@@ skipping 18 matching lines @@
       // is so a broken transformer doesn't take down the whole graph.
       phase.cascade.reportError(_wrapException(error, stackTrace));
 
       return false;
     }).then((isPrimary) {
       if (_isRemoved) return null;
       if (isPrimary) {
         if (!deferred) primary.force();
         return _declareOutputs().then((_) {
           if (_isRemoved) return;
-          if (_awaitingForce) {
+          if (_forced) {
+            _apply();
+          } else {
             _state = _State.DECLARED;
             _onDoneController.add(null);
-          } else {
-            _apply();
           }
         });
       }
 
       _emitPassThrough();
       _state = _State.NOT_PRIMARY;
       _onDoneController.add(null);
     });
   }
 
@@ skipping 16 matching lines @@
           .where((id) => id.package != phase.cascade.package).toSet();
       for (var id in invalidIds) {
         _declaredOutputs.remove(id);
         // TODO(nweiz): report this as a warning rather than a failing error.
         phase.cascade.reportError(new InvalidOutputException(info, id));
       }
 
       if (!_declaredOutputs.contains(primary.id)) _emitPassThrough();
 
       for (var id in _declaredOutputs) {
-        var controller = _awaitingForce
-            ? new AssetNodeController.lazy(id, force, this)
-            : new AssetNodeController(id, this);
+        var controller = _forced
+            ? new AssetNodeController(id, this)
+            : new AssetNodeController.lazy(id, force, this);
         _outputControllers[id] = controller;
         _onAssetController.add(controller.node);
       }
     }).catchError((error, stackTrace) {
       if (_isRemoved) return;
       phase.cascade.reportError(_wrapException(error, stackTrace));
     });
   }
 
   /// Applies this transform.
   void _apply() {
-    assert(!_isRemoved && !_awaitingForce);
+    assert(!_isRemoved);
 
     // Clear input subscriptions here as well as in [_process] because [_apply]
     // may be restarted independently if only a secondary input changes.
     _clearInputSubscriptions();
     _state = _State.APPLYING;
     _runApply().then((hadError) {
       if (_isRemoved) return;
 
+      if (_state == _State.DECLARED) return;
+
       if (_state == _State.NEEDS_APPLY) {
         _apply();
         return;
       }
 
+      if (deferred) _forced = false;
+
       assert(_state == _State.APPLYING);
       if (hadError) {
         _clearOutputs();
         // If the transformer threw an error, we don't want to emit the
         // pass-through asset in case it will be overwritten by the transformer.
         // However, if the transformer declared that it wouldn't overwrite or
         // consume the pass-through asset, we can safely emit it.
         if (_declaredOutputs != null && !_consumePrimary &&
             !_declaredOutputs.contains(primary.id)) {
           _emitPassThrough();
@@ skipping 33 matching lines @@
   /// Returns whether or not an error occurred while running the transformer.
   Future<bool> _runApply() {
     var transformController = new TransformController(this);
     _onLogPool.add(transformController.onLog);
 
     return primary.whenAvailable((_) {
       if (_isRemoved) return null;
       _state = _State.APPLYING;
       return syncFuture(() => transformer.apply(transformController.transform));
     }).then((_) {
-      if (_state == _State.NEEDS_APPLY || _isRemoved) return false;
+      if (deferred && !_forced && !primary.state.isAvailable) {
+        _state = _State.DECLARED;
+        _onDoneController.add(null);
+        return false;
+      }
+
+      if (_isRemoved) return false;
+      if (_state == _State.NEEDS_APPLY) return false;
+      if (_state == _State.DECLARING) return false;
       if (transformController.loggedError) return true;
       _handleApplyResults(transformController);
       return false;
     }).catchError((error, stackTrace) {
       // If the transform became dirty while processing, ignore any errors from
       // it.
       if (_state == _State.NEEDS_APPLY || _isRemoved) return false;
 
       // Catch all transformer errors and pipe them to the results stream. This
       // is so a broken transformer doesn't take down the whole graph.
@@ skipping 156 matching lines @@
   ///
   /// This will never transition to another state.
   static final NOT_PRIMARY = const _State._("not primary");
 
   final String name;
 
   const _State._(this.name);
 
   String toString() => name;
 }