Domain fix

src/compile/data/index.ts

@@ -35,6 +35,13 @@ export interface DataComponent {
    */
   facetRoot?: FacetNode;
 
+  /**
+   * True if the data for this model is faceted.
+   * A dataset is faceted if a parent model is a facet and no new dataset is
+   * defined (which would make the data unfaceted again).
+   */
+  isFaceted: boolean;
+
   /**
    * Parse properties passed down from ancestors.
    */

src/compile/data/optimize.ts

@@ -81,7 +81,7 @@ function moveMainDownToFacet(node: DataFlowNode) {
 }
 
 /**
- * Start optimization path from the root. Useful for removing nodes.
+ * Remove nodes that are not required starting from a root.
  */
 function removeUnnecessaryNodes(node: DataFlowNode) {
 

src/compile/data/optimizers.ts

@@ -1,5 +1,6 @@
 import {hasIntersection, keys} from '../../util';
 import {DataFlowNode, OutputNode} from './dataflow';
+import {FacetNode} from './facet';
 import {ParseNode} from './formatparse';
 import {SourceNode} from './source';
 import {TimeUnitNode} from './timeunit';
@@ -58,11 +59,12 @@ export function moveParseUp(node: DataFlowNode) {
 }
 
 /**
- * Repeatedly remove leaf nodes that are not output nodes.
+ * Repeatedly remove leaf nodes that are not output or facet nodes.
  * The reason is that we don't need subtrees that don't have any output nodes.
+ * Facet nodes are needed for the row or column domains.
  */
 export function removeUnusedSubtrees(node: DataFlowNode) {
-  if (node instanceof OutputNode || node.numChildren() > 0) {
+  if (node instanceof OutputNode || node.numChildren() > 0 || node instanceof FacetNode) {
     // no need to continue with parent because it is output node or will have children (there was a fork)
     return false;
   } else {

src/compile/model.ts

@@ -8,7 +8,7 @@ import {ChannelDef, FieldDef, FieldRefOption, getFieldDef, vgField} from '../fie
 import * as log from '../log';
 import {Resolve} from '../resolve';
 import {hasDiscreteDomain} from '../scale';
-import {BaseSpec} from '../spec';
+import {BaseSpec, isFacetSpec} from '../spec';
 import {extractTitleConfig, TitleParams} from '../title';
 import {normalizeTransform, Transform} from '../transform';
 import {contains, Dict, keys, varName} from '../util';
@@ -27,7 +27,7 @@ import {
 import {assembleAxes} from './axis/assemble';
 import {AxisComponentIndex} from './axis/component';
 import {ConcatModel} from './concat';
-import {DataComponent} from './data/index';
+import {DataComponent} from './data';
 import {FacetModel} from './facet';
 import {LayerModel} from './layer';
 import {getHeaderGroups, getTitleGroup, HEADER_CHANNELS, LayoutHeaderComponent} from './layout/header';
@@ -186,7 +186,9 @@ export abstract class Model {
         sources: parent ? parent.component.data.sources : {},
         outputNodes: parent ? parent.component.data.outputNodes : {},
         outputNodeRefCounts: parent ? parent.component.data.outputNodeRefCounts : {},
-        ancestorParse: parent ? {...parent.component.data.ancestorParse} : {}
+        ancestorParse: parent ? {...parent.component.data.ancestorParse} : {},
+        // data is faceted if the spec is a facet spec or the parent has faceted data and no data is defined
+        isFaceted: isFacetSpec(spec) || (parent && parent.component.data.isFaceted && !spec.data)
       },
       layoutSize: new Split<LayoutSizeIndex>(),
       layoutHeaders:{row: {}, column: {}},

src/compile/scale/domain.ts

@@ -5,19 +5,20 @@ import {MAIN, RAW} from '../../data';
 import {DateTime, dateTimeExpr, isDateTime} from '../../datetime';
 import {FieldDef} from '../../fielddef';
 import * as log from '../../log';
+import {ResolveMode} from '../../resolve';
 import {Domain, hasDiscreteDomain, isBinScale, isSelectionDomain, ScaleConfig, ScaleType} from '../../scale';
 import {isSortField, SortField} from '../../sort';
 import * as util from '../../util';
+import {isDataRefUnionedDomain, isFieldRefUnionDomain} from '../../vega.schema';
 import {
   isDataRefDomain,
   VgDataRef,
   VgDomain,
   VgFieldRefUnionDomain,
   VgNonUnionDomain,
   VgSortField,
-  VgUnionSortField
+  VgUnionSortField,
 } from '../../vega.schema';
-import {isDataRefUnionedDomain, isFieldRefUnionDomain} from '../../vega.schema';
 import {binRequiresRange} from '../common';
 import {FACET_SCALE_PREFIX} from '../data/optimize';
 import {isFacetModel, isUnitModel, Model} from '../model';
@@ -57,6 +58,26 @@ function parseUnitScaleDomain(model: UnitModel) {
         signal: SELECTION_DOMAIN + JSON.stringify(specifiedDomain)
       }, true);
     }
+
+    if (model.component.data.isFaceted) {
+      // get resolve from closest facet parent as this decides whether we need to refer to cloned subtree or not
+      let facetParent: Model = model;
+      while (!isFacetModel(facetParent) && facetParent.parent) {
+        facetParent = facetParent.parent;
+      }
+
+      const resolve = facetParent.component.resolve.scale[channel];
+
+      if (resolve === 'shared') {
+        for (const domain of domains) {
+          // Replace the scale domain with data output from a cloned subtree after the facet.
+          if (isDataRefDomain(domain)) {
+            // use data from cloned subtree (which is the same as data but with a prefix added once)
+            domain.data = FACET_SCALE_PREFIX + domain.data.replace(FACET_SCALE_PREFIX, '');
+          }
+        }
+      }
+    }
   });
 }
 
@@ -83,16 +104,6 @@ function parseNonUnitScaleDomain(model: Model) {
       }
     }
 
-    if (isFacetModel(model)) {
-      domains.forEach((domain) => {
-        // Replace the scale domain with data output from a cloned subtree after the facet.
-        if (isDataRefDomain(domain)) {
-          // use data from cloned subtree (which is the same as data but with a prefix added once)
-          domain.data = FACET_SCALE_PREFIX + domain.data.replace(FACET_SCALE_PREFIX, '');
-        }
-      });
-    }
-
     localScaleComponents[channel].domains = domains;
   });
 }

src/compile/split.ts

@@ -1,6 +1,6 @@
 
 import * as log from '../log';
-import {duplicate} from '../util';
+import {duplicate, hash} from '../util';
 
 /**
  * Generic class for storing properties that are explicitly specified
@@ -101,7 +101,7 @@ export function tieBreakByComparing<S, T>(compare: (v1: T, v2: T) => number) {
 
 export function defaultTieBreaker<S, T>(v1: Explicit<T>, v2: Explicit<T>, property: keyof S, propertyOf: string) {
   if (v1.explicit && v2.explicit) {
-    log.warn(log.message.mergeConflictingProperty(property, propertyOf, v1.value, v2.value));
+    log.warn(log.message.mergeConflictingProperty(property, propertyOf, hash(v1.value), hash(v2.value)));
   }
   // If equal score, prefer v1.
   return v1;

src/spec.ts

@@ -208,31 +208,31 @@ export type TopLevelExtendedSpec = TopLevel<FacetedCompositeUnitSpec> | TopLevel
 /* Custom type guards */
 
 
-export function isFacetSpec(spec: GenericSpec<GenericUnitSpec<any, any>>): spec is GenericFacetSpec<GenericUnitSpec<any, any>> {
+export function isFacetSpec(spec: BaseSpec): spec is GenericFacetSpec<GenericUnitSpec<any, any>> {
   return spec['facet'] !== undefined;
 }
 
-export function isUnitSpec(spec: GenericSpec<GenericUnitSpec<any, any>>): spec is FacetedCompositeUnitSpec | UnitSpec {
+export function isUnitSpec(spec: BaseSpec): spec is FacetedCompositeUnitSpec | UnitSpec {
   return !!spec['mark'];
 }
 
-export function isLayerSpec(spec: GenericSpec<GenericUnitSpec<any, any>>): spec is GenericLayerSpec<GenericUnitSpec<any, any>> {
+export function isLayerSpec(spec: BaseSpec): spec is GenericLayerSpec<GenericUnitSpec<any, any>> {
   return spec['layer'] !== undefined;
 }
 
-export function isRepeatSpec(spec: GenericSpec<GenericUnitSpec<any, any>>): spec is GenericRepeatSpec<GenericUnitSpec<any, any>> {
+export function isRepeatSpec(spec: BaseSpec): spec is GenericRepeatSpec<GenericUnitSpec<any, any>> {
   return spec['repeat'] !== undefined;
 }
 
-export function isConcatSpec(spec: GenericSpec<GenericUnitSpec<any, any>>): spec is GenericVConcatSpec<GenericUnitSpec<any, any>> | GenericHConcatSpec<GenericUnitSpec<any, any>> {
+export function isConcatSpec(spec: BaseSpec): spec is GenericVConcatSpec<GenericUnitSpec<any, any>> | GenericHConcatSpec<GenericUnitSpec<any, any>> {
   return isVConcatSpec(spec) || isHConcatSpec(spec);
 }
 
-export function isVConcatSpec(spec: GenericSpec<GenericUnitSpec<any, any>>): spec is GenericVConcatSpec<GenericUnitSpec<any, any>> {
+export function isVConcatSpec(spec: BaseSpec): spec is GenericVConcatSpec<GenericUnitSpec<any, any>> {
   return spec['vconcat'] !== undefined;
 }
 
-export function isHConcatSpec(spec: GenericSpec<GenericUnitSpec<any, any>>): spec is GenericHConcatSpec<GenericUnitSpec<any, any>> {
+export function isHConcatSpec(spec: BaseSpec): spec is GenericHConcatSpec<GenericUnitSpec<any, any>> {
   return spec['hconcat'] !== undefined;
 }
 

test/compile/data/assemble.test.ts

@@ -21,7 +21,8 @@ describe('compile/data/assemble', () => {
         sources: {named: src},
         outputNodes: {out: main},
         outputNodeRefCounts,
-        ancestorParse: {}
+        ancestorParse: {},
+        isFaceted: false
       });
 
       assert.equal(data.length, 1);
@@ -45,7 +46,8 @@ describe('compile/data/assemble', () => {
         sources: {named: src},
         outputNodes: {out: main},
         outputNodeRefCounts,
-        ancestorParse: {}
+        ancestorParse: {},
+        isFaceted: false
       });
 
       assert.deepEqual<VgData[]>(data, [{

test/compile/scale/parse.test.ts

@@ -1,12 +1,13 @@
 /* tslint:disable:quotemark */
 
 import {assert} from 'chai';
+
 import {parseScaleCore} from '../../../src/compile/scale/parse';
 import {SELECTION_DOMAIN} from '../../../src/compile/selection/selection';
 import * as log from '../../../src/log';
 import {NON_TYPE_DOMAIN_RANGE_VEGA_SCALE_PROPERTIES, SCALE_PROPERTIES} from '../../../src/scale';
 import {toSet, without} from '../../../src/util';
-import {parseModel, parseUnitModelWithScale} from '../../util';
+import {parseModel, parseModelWithScale, parseUnitModelWithScale} from '../../util';
 
 describe('src/compile', function() {
   it('NON_TYPE_RANGE_SCALE_PROPERTIES should be SCALE_PROPERTIES wihtout type, domain, and range properties', () => {
@@ -352,6 +353,7 @@ describe('src/compile', function() {
 
       const xScale = model.getScaleComponent('x');
       const yscale = model.getScaleComponent('y');
+
       it('should add a raw selection domain', function() {
         assert.property(xScale.explicit, 'domainRaw');
         assert.propertyVal(xScale.explicit.domainRaw, 'signal',
@@ -363,4 +365,71 @@ describe('src/compile', function() {
       });
     });
   });
+
+  describe('parseScaleDomain', function() {
+    describe('faceted domains', function() {
+      it('should use cloned subtree', function() {
+        const model = parseModelWithScale({
+          facet: {
+            row: {field: "symbol", type: "nominal"}
+          },
+          spec: {
+            mark: 'point',
+            encoding: {
+              x: {field: 'a', type: 'quantitative'},
+            }
+          }
+        });
+
+        assert.deepEqual(model.component.scales.x.domains, [{
+          data: 'scale_child_main',
+          field: 'a'
+        }]);
+      });
+
+      it('should not use cloned subtree if the data is not faceted', function() {
+        const model = parseModelWithScale({
+          facet: {
+            row: {field: "symbol", type: "nominal"}
+          },
+          spec: {
+            data: {url: 'foo'},
+            mark: 'point',
+            encoding: {
+              x: {field: 'a', type: 'quantitative'},
+            }
+          }
+        });
+
+        assert.deepEqual(model.component.scales.x.domains, [{
+          data: 'child_main',
+          field: 'a'
+        }]);
+      });
+
+      it('should not use cloned subtree if the scale is independent', function() {
+        const model = parseModelWithScale({
+          facet: {
+            row: {field: "symbol", type: "nominal"}
+          },
+          spec: {
+            mark: 'point',
+            encoding: {
+              x: {field: 'a', type: 'quantitative'},
+            }
+          },
+          resolve: {
+            scale: {
+              x: 'independent'
+            }
+          }
+        });
+
+        assert.deepEqual(model.children[0].component.scales.x.domains, [{
+          data: 'child_main',
+          field: 'a'
+        }]);
+      });
+    });
+  });
 });