improve top-level field codegen: avoid Element.node getter

also correctly exports the values of top-level fields, and ensures it uses the most up-to-date value for mutable top-level fields

we can likely improve this further once we have ES6 modules, but it seems like an okay start

[email protected]

Review URL: https://chromereviews.googleplex.com/147927013

Author: John Messerly

lib/src/codegen/js_codegen.dart

@@ -79,6 +79,7 @@ var $libName;
 
     if (_exports.isNotEmpty) out.write('// Exports:\n');
 
+    // TODO(jmesserly): make these immutable?
     for (var name in _exports) {
       out.write('${libraryInfo.name}.$name = $name;\n');
     }
@@ -302,13 +303,10 @@ var $libName;
     var unsetFields = new Map<String, Expression>();
     for (var declaration in fields) {
       for (var field in declaration.fields.variables) {
-        if (!_isConstantField(field)) {
-          field.name.accept(this);
-          out.write(' = ');
-          field.initializer.accept(this);
-          out.write(';\n');
-        } else {
+        if (_isFieldInitConstant(field)) {
           unsetFields[field.name.name] = field.initializer;
+        } else {
+          _visitNode(field, suffix: ';\n');
         }
       }
     }
@@ -611,29 +609,39 @@ var $libName;
 
   @override
   void visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) {
-    _visitNode(node.variables);
+    var constFields = <VariableDeclaration>[];
+    for (var field in node.variables.variables) {
+      var name = field.name.name;
+      if (field.isConst) {
+        // constant fields don't change, so we can generate them as `let`
+        // but add them to the module's exports
+        _visitNode(field, prefix: 'let ', suffix: ';\n');
+        if (isPublic(name)) _exports.add(name);
+      } else if (_isFieldInitConstant(field)) {
+        _visitNode(field, suffix: ';\n');
+      } else {
+        _lazyFields.add(field);
+      }
+    }
   }
 
   @override
   void visitVariableDeclarationList(VariableDeclarationList node) {
-    var topLevel = node.parent is TopLevelVariableDeclaration;
-
-    var declarations = node.variables;
-
-    var wroteLet = false;
-    for (var node in declarations) {
-      if (topLevel && !_isConstantField(node)) {
-        _lazyFields.add(node);
-        continue;
-      }
+    _visitNodeList(node.variables, prefix: 'let ', separator: ', ');
+  }
 
-      wroteLet = true;
-      out.write(node == declarations.first ? 'let ' : ', ');
-      out.write(node.name.name);
-      _visitNode(node.initializer, prefix: ' = ');
+  @override
+  void visitVariableDeclaration(VariableDeclaration node) {
+    node.name.accept(this);
+    out.write(' = ');
+    if (node.initializer != null) {
+      node.initializer.accept(this);
+    } else {
+      // explicitly initialize to null, so we don't need to worry about
+      // `undefined`.
+      // TODO(jmesserly): do this only for vars that aren't definitely assigned.
+      out.write('null');
     }
-
-    if (topLevel && wroteLet) out.write(';\n');
   }
 
   void _flushLazyFields() {
@@ -1013,7 +1021,7 @@ var $libName;
     out.write('/* Unimplemented ${node.runtimeType}: $node */');
   }
 
-  bool _isConstantField(VariableDeclaration field) =>
+  bool _isFieldInitConstant(VariableDeclaration field) =>
       field.initializer == null || _computeConstant(field) is ValidResult;
 
   EvaluationResultImpl _computeConstant(VariableDeclaration field) {
@@ -1051,11 +1059,7 @@ var $libName;
     if (element is PropertyAccessorElement) {
       element = (element as PropertyAccessorElement).variable;
     }
-    return element is TopLevelVariableElement &&
-        // TODO(sigmund): refactor so we can remove `.node` here. This may
-        // require tracking in our checker for const values, so we can look it
-        // up cheaply here.
-        !_isConstantField(element.node);
+    return element is TopLevelVariableElement && !element.isConst;
   }
 
   /// Safely visit the given node, with an optional prefix or suffix.

test/codegen/expect/DeltaBlue/DeltaBlue.dart.js

@@ -49,7 +49,7 @@ var DeltaBlue;
     }
     addConstraint() {
       this.addToGraph();
-      planner.incrementalAdd(this);
+      DeltaBlue.planner.incrementalAdd(this);
     }
     satisfy(mark) {
       this.chooseMethod(/* Unimplemented: DownCast: dynamic to int */ mark);
@@ -64,12 +64,12 @@ var DeltaBlue;
       let overridden = out.determinedBy;
       if (overridden !== null) overridden.this.markUnsatisfied();
       out.determinedBy = this;
-      if (!planner.addPropagate(this, /* Unimplemented: DownCast: dynamic to int */ mark)) dart_core.print("Cycle encountered");
+      if (!DeltaBlue.planner.addPropagate(this, /* Unimplemented: DownCast: dynamic to int */ mark)) dart_core.print("Cycle encountered");
       out.mark = /* Unimplemented: DownCast: dynamic to int */ mark;
       return overridden;
     }
     destroyConstraint() {
-      if (this.isSatisfied()) planner.incrementalRemove(this);
+      if (this.isSatisfied()) DeltaBlue.planner.incrementalRemove(this);
       this.removeFromGraph();
     }
     isInput() { return false; }
@@ -355,7 +355,7 @@ var DeltaBlue;
 
   // Function chainTest: (int) → void
   function chainTest(n) {
-    planner = new Planner();
+    DeltaBlue.planner = new Planner();
     let prev = null, first = null, last = null;
     for (let i = 0; i <= n; i++) {
       let v = new Variable("v", 0);
@@ -366,7 +366,7 @@ var DeltaBlue;
     }
     new StayConstraint(last, STRONG_DEFAULT);
     let edit = new EditConstraint(first, PREFERRED);
-    let plan = planner.extractPlanFromConstraints(/* Unimplemented ArrayList */[edit]);
+    let plan = DeltaBlue.planner.extractPlanFromConstraints(/* Unimplemented ArrayList */[edit]);
     for (let i = 0; i < 100; i++) {
       first.value = i;
       plan.execute();
@@ -379,7 +379,7 @@ var DeltaBlue;
 
   // Function projectionTest: (int) → void
   function projectionTest(n) {
-    planner = new Planner();
+    DeltaBlue.planner = new Planner();
     let scale = new Variable("scale", 10);
     let offset = new Variable("offset", 1000);
     let src = null, dst = null;
@@ -408,23 +408,33 @@ var DeltaBlue;
   // Function change: (Variable, int) → void
   function change(v, newValue) {
     let edit = new EditConstraint(v, PREFERRED);
-    let plan = planner.extractPlanFromConstraints(/* Unimplemented ArrayList */[edit]);
+    let plan = DeltaBlue.planner.extractPlanFromConstraints(/* Unimplemented ArrayList */[edit]);
     for (let i = 0; i < 10; i++) {
       v.value = newValue;
       plan.execute();
     }
     edit.destroyConstraint();
   }
 
-  let planner;
+  DeltaBlue.planner = null;
   // Exports:
   DeltaBlue.main = main;
   DeltaBlue.DeltaBlue = DeltaBlue;
   DeltaBlue.Strength = Strength;
+  DeltaBlue.REQUIRED = REQUIRED;
+  DeltaBlue.STRONG_PREFERRED = STRONG_PREFERRED;
+  DeltaBlue.PREFERRED = PREFERRED;
+  DeltaBlue.STRONG_DEFAULT = STRONG_DEFAULT;
+  DeltaBlue.NORMAL = NORMAL;
+  DeltaBlue.WEAK_DEFAULT = WEAK_DEFAULT;
+  DeltaBlue.WEAKEST = WEAKEST;
   DeltaBlue.Constraint = Constraint;
   DeltaBlue.UnaryConstraint = UnaryConstraint;
   DeltaBlue.StayConstraint = StayConstraint;
   DeltaBlue.EditConstraint = EditConstraint;
+  DeltaBlue.NONE = NONE;
+  DeltaBlue.FORWARD = FORWARD;
+  DeltaBlue.BACKWARD = BACKWARD;
   DeltaBlue.BinaryConstraint = BinaryConstraint;
   DeltaBlue.ScaleConstraint = ScaleConstraint;
   DeltaBlue.EqualityConstraint = EqualityConstraint;

test/codegen/expect/dom/dom.dart.js

@@ -19,7 +19,7 @@ var dom;
   }
 
   let overload = new Overload();
-  let document;
+  dom.document = null;
   class Document {
   }
 
@@ -93,6 +93,7 @@ var dom;
   dom.JsType = JsType;
   dom.JsGlobal = JsGlobal;
   dom.Overload = Overload;
+  dom.overload = overload;
   dom.Document = Document;
   dom.Element = Element;
   dom.Event = Event;

test/codegen/expect/sunflower/sunflower.dart.js

@@ -11,7 +11,7 @@ var sunflower;
   // Function querySelector: (String) → Element
   function querySelector(selector) { return dom.document.querySelector(selector); }
 
-  let seeds = 0;
+  sunflower.seeds = 0;
   dart.defineLazyProperties(sunflower, {
     get slider() { return /* Unimplemented: DownCast: Element to InputElement */ querySelector("#slider") },
     get notes() { return querySelector("#notes") },
@@ -52,19 +52,26 @@ var sunflower;
 
   // Function draw: () → void
   function draw() {
-    seeds = dart_core.int.parse(sunflower.slider.value);
+    sunflower.seeds = dart_core.int.parse(sunflower.slider.value);
     sunflower.context.clearRect(0, 0, MAX_D, MAX_D);
-    for (let i = 0; i < seeds; i++) {
+    for (let i = 0; i < sunflower.seeds; i++) {
       let theta = i * TAU / sunflower.PHI;
       let r = dart_math.sqrt(i) * SCALE_FACTOR;
       let x = centerX + r * dart_math.cos(theta);
       let y = centerY - r * dart_math.sin(theta);
       new SunflowerSeed(x, y).draw();
     }
-    sunflower.notes.textContent = "" + (seeds) + " seeds";
+    sunflower.notes.textContent = "" + (sunflower.seeds) + " seeds";
   }
 
   // Exports:
+  sunflower.ORANGE = ORANGE;
+  sunflower.SEED_RADIUS = SEED_RADIUS;
+  sunflower.SCALE_FACTOR = SCALE_FACTOR;
+  sunflower.TAU = TAU;
+  sunflower.MAX_D = MAX_D;
+  sunflower.centerX = centerX;
+  sunflower.centerY = centerY;
   sunflower.querySelector = querySelector;
   sunflower.Circle = Circle;
   sunflower.SunflowerSeed = SunflowerSeed;