Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: break cyclic dependency between flowBuilder and UnitView #193

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

refactor: break cyclic dependency between flowBuilder and UnitView #193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/core/src/data/sources/dynamic/singleAxisLazySource.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import UnitView from "../../../view/unitView.js";
import DataSource from "../dataSource.js";
import { reconfigureScales } from "../../../view/scaleResolution.js";

Expand Down Expand Up @@ -33,7 +32,9 @@ export default class SingleAxisLazySource extends DataSource {
const sentences = [
`The dynamic data source cannot find a resolved scale for channel "${channel}".`,
];
if (!(this.view instanceof UnitView)) {

// equivalent to "view instanceof UnitView"
if (!("mark" in view)) {
sentences.push(
`Make sure the view has a "shared" scale resolution as it is not a unit view.`
);
Expand Down
21 changes: 13 additions & 8 deletions packages/core/src/view/flowBuilder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import Collector from "../data/collector.js";
import createTransform from "../data/transforms/transformFactory.js";
import createDataSource from "../data/sources/dataSourceFactory.js";
import UnitView from "./unitView.js";
import { BEHAVIOR_MODIFIES } from "../data/flowNode.js";
import CloneTransform from "../data/transforms/clone.js";
import DataFlow from "../data/dataFlow.js";
Expand Down Expand Up @@ -128,15 +127,21 @@ export function buildDataFlow(root, existingFlow) {
createTransforms(view.spec.transform, view);
}

if (view instanceof UnitView) {
// The following is equivalent to "view instanceof UnitView" but without dependency.
// It's a bit fragile and should be replaced with a proper type guard.
if ("mark" in view) {
const unitView = /** @type {import("./unitView.js").default}*/ (
view
);

if (!currentNode) {
throw new Error(
`A unit view (${view.getPathString()}) has no (inherited) data source`
`A unit view (${unitView.getPathString()}) has no (inherited) data source`
);
}

// Support chrom/pos channelDefs
const linearize = linearizeLocusAccess(view);
const linearize = linearizeLocusAccess(unitView);
if (linearize) {
postProcessOps.push(linearize.rewrite);
for (const transform of linearize.transforms) {
Expand All @@ -147,7 +152,7 @@ export function buildDataFlow(root, existingFlow) {
}
}

if (view.mark.isPickingParticipant()) {
if (unitView.mark.isPickingParticipant()) {
appendTransform(new CloneTransform());
appendTransform(
new IdentifierTransform({ type: "identifier" })
Expand All @@ -156,15 +161,15 @@ export function buildDataFlow(root, existingFlow) {

const collector = new Collector({
type: "collect",
groupby: view.getFacetFields(),
groupby: unitView.getFacetFields(),
sort: getCompareParamsForView(
view,
unitView,
linearize?.rewrittenEncoding
),
});

appendNode(collector);
dataFlow.addCollector(collector, view);
dataFlow.addCollector(collector, unitView);
}
};

Expand Down