Skip to content

Commit

Permalink
Prevent duplicate signals from being generated for top-level LayerMod…
Browse files Browse the repository at this point in the history
…el. (#5350)

When a LayerModel is at the top-level, top-level signals and unit model
signals all get flattened into the same array. As a result, we would
have bound projection signals alongside their "push: outer" siblings.
Although technically incorrect, this approach worked fine up until
Vega 5.5, when a change to the dataflow was introduced.
  • Loading branch information
arvind authored Aug 28, 2019
1 parent 9619241 commit d13245f
Showing 1 changed file with 7 additions and 2 deletions.
9 changes: 7 additions & 2 deletions src/compile/selection/transforms/scales.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {accessPathWithDatum, varName} from '../../../util';
import {UnitModel} from '../../unit';
import {SelectionProjection} from './project';
import {TransformCompiler} from './transforms';
import {isLayerModel, Model} from '../../model';

const scaleBindings: TransformCompiler = {
has: selCmpt => {
Expand Down Expand Up @@ -48,7 +49,7 @@ const scaleBindings: TransformCompiler = {

// Top-level signals are only needed for multiview displays and if this
// view's top-level signals haven't already been generated.
if (!model.parent || !bound.length) {
if (!model.parent || isTopLevelLayer(model) || !bound.length) {
return signals;
}

Expand Down Expand Up @@ -77,7 +78,7 @@ const scaleBindings: TransformCompiler = {

signals: (model, selCmpt, signals) => {
// Nested signals need only push to top-level signals with multiview displays.
if (model.parent) {
if (model.parent && !isTopLevelLayer(model)) {
for (const proj of selCmpt.scales) {
const signal: any = signals.filter(s => s.name === proj.signals.data)[0];
signal.push = 'outer';
Expand All @@ -96,3 +97,7 @@ export function domain(model: UnitModel, channel: Channel) {
const scale = stringValue(model.scaleName(channel));
return `domain(${scale})`;
}

function isTopLevelLayer(model: Model): boolean {
return model.parent && isLayerModel(model.parent) && (!model.parent.parent || isTopLevelLayer(model.parent.parent));
}

0 comments on commit d13245f

Please sign in to comment.