Replace forEach with for, remove as, remove push.apply, add test, and…

… initialize fill array (#4216)

* Replace forEach with for, remove as, add tests and initialize fill array

* Get rid of push.apply

* Unused import

* Replace more forEach with for..of.

src/compile/selection/interval.ts

@@ -53,7 +53,7 @@ const interval: SelectionCompiler = {
       const scaleType = model.getScaleComponent(channel).get('type');
       const toNum = hasContinuousDomain(scaleType) ? '+' : '';
 
-      signals.push.apply(signals, cs);
+      signals.push(...cs);
       dataSignals.push(dname);
 
       scaleTriggers.push({

src/compile/selection/selection.ts

@@ -119,7 +119,7 @@ export function assembleUnitSelectionSignals(model: UnitModel, signals: any[]) {
     const name = selCmpt.name;
     let modifyExpr = selCompiler.modifyExpr(model, selCmpt);
 
-    signals.push.apply(signals, selCompiler.signals(model, selCmpt));
+    signals.push(...selCompiler.signals(model, selCmpt));
 
     forEachTransform(selCmpt, txCompiler => {
       if (txCompiler.signals) {
@@ -228,11 +228,11 @@ export function assembleUnitSelectionMarks(model: UnitModel, marks: any[]): any[
 }
 
 export function assembleLayerSelectionMarks(model: LayerModel, marks: any[]): any[] {
-  model.children.forEach(child => {
+  for (const child of model.children) {
     if (isUnitModel(child)) {
       marks = assembleUnitSelectionMarks(child, marks);
     }
-  });
+  }
 
   return marks;
 }

src/compile/selection/transforms/inputs.ts

@@ -15,7 +15,7 @@ const inputBindings: TransformCompiler = {
     const bind = selCmpt.bind;
     const datum = nearest.has(selCmpt) ? '(item().isVoronoi ? datum.datum : datum)' : 'datum';
 
-    proj.forEach(p => {
+    for (const p of proj) {
       const sgname = varName(`${name}_${p.field}`);
       const hasSignal = signals.filter(s => s.name === sgname);
       if (!hasSignal.length) {
@@ -31,7 +31,7 @@ const inputBindings: TransformCompiler = {
           bind: bind[p.field] || bind[p.channel] || bind
         });
       }
-    });
+    }
 
     return signals;
   },

src/compile/selection/transforms/project.ts

@@ -1,4 +1,4 @@
-import {ScaleChannel, SingleDefChannel} from '../../../channel';
+import {ScaleChannel} from '../../../channel';
 import * as log from '../../../log';
 import {hasContinuousDomain, isBinScale} from '../../../scale';
 import {SelectionDef} from '../../../selection';
@@ -23,10 +23,10 @@ const project: TransformCompiler = {
 
     // TODO: find a possible channel mapping for these fields.
     if (selDef.fields) {
-      p.push.apply(p, selDef.fields.map(field => ({field, type: 'E'})));
+      p.push(...selDef.fields.map<ProjectSelectionComponent>(field => ({field, type: 'E'})));
     }
 
-    (selDef.encodings || []).forEach((channel: SingleDefChannel) => {
+    for (const channel of selDef.encodings || []) {
       const fieldDef = model.fieldDef(channel);
       if (fieldDef) {
         let field = fieldDef.field;
@@ -67,7 +67,7 @@ const project: TransformCompiler = {
       } else {
         log.warn(log.message.cannotProjectOnChannelWithoutField(channel));
       }
-    });
+    }
 
     if (keys(timeUnits).length) {
       selCmpt.timeUnit = new TimeUnitNode(null, timeUnits);

src/compile/selection/transforms/scales.ts

@@ -1,5 +1,5 @@
 import {stringValue} from 'vega-util';
-import {Channel, ScaleChannel, X, Y} from '../../../channel';
+import {Channel, isScaleChannel, X, Y} from '../../../channel';
 import * as log from '../../../log';
 import {hasContinuousDomain, isBinScale} from '../../../scale';
 import {accessPathWithDatum, varName} from '../../../util';
@@ -16,14 +16,19 @@ const scaleBindings: TransformCompiler = {
     const name = varName(selCmpt.name);
     const bound: Channel[] = (selCmpt.scales = []);
 
-    selCmpt.project.forEach(p => {
-      const channel = p.channel as ScaleChannel;
+    for (const p of selCmpt.project) {
+      const channel = p.channel;
+
+      if (!isScaleChannel(channel)) {
+        continue;
+      }
+
       const scale = model.getScaleComponent(channel);
       const scaleType = scale ? scale.get('type') : undefined;
 
       if (!scale || !hasContinuousDomain(scaleType) || isBinScale(scaleType)) {
         log.warn(log.message.SCALE_BINDINGS_CONTINUOUS);
-        return;
+        continue;
       }
 
       scale.set('domainRaw', {signal: accessPathWithDatum(p.field, name)}, true);
@@ -34,7 +39,7 @@ const scaleBindings: TransformCompiler = {
         const scale2 = model.getScaleComponent(channel === X ? Y : X);
         scale2.set('domainRaw', {signal: accessPathWithDatum(p.field, name)}, true);
       }
-    });
+    }
   },
 
   topLevelSignals: (model, selCmpt, signals) => {
@@ -77,13 +82,13 @@ const scaleBindings: TransformCompiler = {
   signals: (model, selCmpt, signals) => {
     // Nested signals need only push to top-level signals with multiview displays.
     if (model.parent) {
-      selCmpt.scales.forEach(channel => {
+      for (const channel of selCmpt.scales) {
         const signal = signals.filter(s => s.name === channelSignalName(selCmpt, channel, 'data'))[0];
 
         signal.push = 'outer';
         delete signal.value;
         delete signal.update;
-      });
+      }
     }
 
     return signals;

src/util.ts

@@ -111,9 +111,9 @@ export function flatten(arrays: any[]) {
 }
 
 export function fill<T>(val: T, len: number) {
-  const arr: T[] = [];
+  const arr = new Array<T>(len);
   for (let i = 0; i < len; ++i) {
-    arr.push(val);
+    arr[i] = val;
   }
   return arr;
 }

test/util.test.ts

@@ -1,5 +1,5 @@
 import {assert} from 'chai';
-import {entries, fieldIntersection, flatAccessWithDatum, prefixGenerator, unique, uniqueId} from '../src/util';
+import {entries, fieldIntersection, fill, flatAccessWithDatum, prefixGenerator, unique, uniqueId} from '../src/util';
 
 import {
   accessPathDepth,
@@ -202,4 +202,12 @@ describe('util', () => {
       expect(uniqueId() === uniqueId()).toBeFalsy();
     });
   });
+
+  describe('fill', () => {
+    it('should return array of right length and filled with the right values', () => {
+      const arr = fill(42, 5);
+      expect(arr).toHaveLength(5);
+      expect(arr).toEqual([42, 42, 42, 42, 42]);
+    });
+  });
 });