Skip to content

Commit

Permalink
Rewrite mainResult selector to avoid re-creating spec if not necessary
Browse files Browse the repository at this point in the history
  • Loading branch information
starry97 authored and kanitw committed Jul 25, 2017
1 parent ac0c64c commit 7ef0578
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 62 deletions.
3 changes: 2 additions & 1 deletion .vscode/settings.json
Expand Up @@ -16,5 +16,6 @@
},
"files.trimTrailingWhitespace": true,
"tslint.autoFixOnSave": true,
"tsimporter.spaceBetweenBraces": false
"tsimporter.spaceBetweenBraces": false,
"tsimporter.filesToExclude": ["build/**", "lib-test/**"]
}
2 changes: 1 addition & 1 deletion src/components/vega-lite/index.tsx
Expand Up @@ -40,7 +40,7 @@ export class VegaLite extends React.PureComponent<VegaLiteProps, any> {
}

protected componentWillReceiveProps(nextProps: VegaLiteProps) {
if (JSON.stringify(this.props.spec) !== JSON.stringify(nextProps.spec)) {
if (this.props.spec !== nextProps.spec) {
this.renderVega(nextProps.spec);
}
// visual.update(nextProps.vegaSpec);
Expand Down
42 changes: 12 additions & 30 deletions src/components/view-pane/index.tsx
@@ -1,65 +1,50 @@
import {getTopSpecQueryItem, SpecQueryGroup} from 'compassql/build/src/model';
import {Query} from 'compassql/build/src/query/query';
import * as React from 'react';
import * as CSSModules from 'react-css-modules';
import {connect} from 'react-redux';
import {Data} from 'vega-lite/build/src/data';
import {OneOfFilter, RangeFilter} from 'vega-lite/build/src/filter';
import {FacetedCompositeUnitSpec} from 'vega-lite/build/src/spec';
import {ActionHandler, createDispatchHandler} from '../../actions/redux-action';
import {ShelfAction} from '../../actions/shelf';
import {State} from '../../models';
import {Bookmark} from '../../models/bookmark';
import {extractPlotObjects, PlotObject} from '../../models/plot';
import {getTransforms, hasWildcards} from '../../models/shelf/spec';
import {selectBookmark, selectData, selectFilters, selectMainResult, selectQuery} from '../../selectors';
import {PlotObject} from '../../models/plot';
import {selectBookmark, selectMainResultForView} from '../../selectors';
import {Plot} from '../plot';
import {PlotList} from '../plot-list';
import * as styles from './view-pane.scss';

export interface ViewPaneProps extends ActionHandler<ShelfAction> {
data: Data;
query: Query;
filters: Array<RangeFilter | OneOfFilter>;
mainResult: SpecQueryGroup<PlotObject>;
mainResult: {
spec: FacetedCompositeUnitSpec,
plots: PlotObject[]
};
bookmark: Bookmark;
}

class ViewPaneBase extends React.PureComponent<ViewPaneProps, {}> {
public render() {
const {bookmark, data, handleAction, filters, query, mainResult} = this.props;

const isSpecific = !hasWildcards(query.spec).hasAnyWildcard;
const {bookmark, handleAction, mainResult} = this.props;

// if there are no results, then nothing to render.
if (!mainResult) {
return null;
}

if (isSpecific) {
const spec = {
// FIXME: include data in the main spec?
data: data,
transform: getTransforms(filters),
...getTopSpecQueryItem(mainResult).spec
};

if (mainResult.spec) {
return (
<div className="pane" styleName="view-pane-specific">
<h2>Specified View</h2>
<Plot handleAction={handleAction} spec={spec} showBookmarkButton={true} bookmark={bookmark}/>
<Plot handleAction={handleAction} spec={mainResult.spec} showBookmarkButton={true} bookmark={bookmark}/>

{/*{JSON.stringify(this.props.query)}
{JSON.stringify(this.props.mainSpec)}*/}
</div>
);
} else {
const plots = extractPlotObjects(mainResult);

return (
<div className="pane" styleName="view-pane-gallery">
<h2>Specified Views</h2>
<PlotList handleAction={handleAction} plots={plots} bookmark={bookmark}/>
<PlotList handleAction={handleAction} plots={mainResult.plots} bookmark={bookmark}/>
</div>
);
}
Expand All @@ -68,11 +53,8 @@ class ViewPaneBase extends React.PureComponent<ViewPaneProps, {}> {
export const ViewPane = connect(
(state: State) => {
return {
data: selectData(state),
query: selectQuery(state),
filters: selectFilters(state),
// FIXME: refactor the flow for this part (we should support asynchrounous request for this too)
mainResult: selectMainResult(state),
mainResult: selectMainResultForView(state),
bookmark: selectBookmark(state)
};
},
Expand Down
1 change: 0 additions & 1 deletion src/lib-voyager.test.ui.tsx
Expand Up @@ -4,7 +4,6 @@
import {mount} from 'enzyme';
import * as React from 'react';
import * as ReactDOM from 'react-dom';

import {Provider} from 'react-redux';
import {App} from './components/app';
import {CreateVoyager} from './lib-voyager';
Expand Down
37 changes: 14 additions & 23 deletions src/reducers/bookmark.ts
@@ -1,5 +1,5 @@
import {Action, BOOKMARK_ADD_PLOT, BOOKMARK_MODIFY_NOTE, BOOKMARK_REMOVE_PLOT} from '../actions';
import {Bookmark, BookmarkItem} from '../models';
import {Bookmark} from '../models';


export function bookmarkReducer(bookmark: Bookmark, action: Action): Bookmark {
Expand All @@ -9,17 +9,15 @@ export function bookmarkReducer(bookmark: Bookmark, action: Action): Bookmark {
switch (action.type) {
case BOOKMARK_ADD_PLOT: {
const {plotObject} = action.payload;
const bookmarkItem: BookmarkItem = {
plotObject: plotObject,
note: '',
};

const specKey = JSON.stringify(plotObject.spec);

return {
dict: {
...dict,
[specKey]: bookmarkItem
[specKey]: {
plotObject: plotObject,
note: '',
}
},
count: count + 1,
list: list.concat([specKey])
Expand All @@ -28,37 +26,30 @@ export function bookmarkReducer(bookmark: Bookmark, action: Action): Bookmark {

case BOOKMARK_MODIFY_NOTE: {
const {note, spec} = action.payload;

const specKey = JSON.stringify(spec);
const modifiedBookmarkItem: BookmarkItem = {
...dict[specKey],
note: note
};

return {
dict: {
...dict,
[specKey]: modifiedBookmarkItem
[specKey]: {
...dict[specKey],
note
}
},
count: count,
list: list.slice()
count,
list
};
}

case BOOKMARK_REMOVE_PLOT: {
const {spec} = action.payload;

const specKey = JSON.stringify(spec);
const newBookmark = {
dict: {
...dict
},
const {[specKey]: _, ...newDict} = dict;
return {
dict: newDict,
count: count - 1,
list: list.filter(item => item !== specKey)
};

delete newBookmark.dict[specKey];
return newBookmark;
}

default: {
Expand Down
65 changes: 59 additions & 6 deletions src/selectors/index.ts
@@ -1,36 +1,50 @@
import {Schema} from 'compassql/build/src/schema';
import {SHORT_WILDCARD} from 'compassql/build/src/wildcard';
import {createSelector} from 'reselect';

import {Shelf, ShelfFieldDef, State, toQuery} from '../models';


// Imports to satisfy --declarations build requirements
// https://github.com/Microsoft/TypeScript/issues/9944
// tslint:disable-next-line:no-unused-variable
import {SpecQueryGroup} from 'compassql/build/src/model';
import {getTopSpecQueryItem, SpecQueryGroup} from 'compassql/build/src/model';
// tslint:disable-next-line:no-unused-variable
import {Query} from 'compassql/build/src/query/query';
// tslint:disable-next-line:no-unused-variable
import {StateWithHistory} from 'redux-undo';
// tslint:disable-next-line:no-unused-variable
import {InlineData, NamedData, UrlData} from 'vega-lite/build/src/data';
import {Selector} from 'reselect/src/reselect';
// tslint:disable-next-line:no-unused-variable
import {BoxPlotDef} from 'vega-lite/build/src/compositemark/boxplot';
// tslint:disable-next-line:no-unused-variable
import {Data, InlineData, NamedData, UrlData} from 'vega-lite/build/src/data';
// tslint:disable-next-line:no-unused-variable
import {EncodingWithFacet} from 'vega-lite/build/src/encoding';
// tslint:disable-next-line:no-unused-variable
import {OneOfFilter, RangeFilter} from 'vega-lite/build/src/filter';
// tslint:disable-next-line:no-unused-variable
import {StateBase, VoyagerConfig} from '../models';
import {MarkDef} from 'vega-lite/build/src/mark';
// tslint:disable-next-line:no-unused-variable
import {FacetedCompositeUnitSpec, GenericUnitSpec} from 'vega-lite/build/src/spec';
// tslint:disable-next-line:no-unused-variable
import {Bookmark} from '../models/bookmark';
// tslint:disable-next-line:no-unused-variable
import {VoyagerConfig} from '../models/config';
// tslint:disable-next-line:no-unused-variable
import {StateBase} from '../models/index';
// tslint:disable-next-line:no-unused-variable
import {PlotObject} from '../models/plot';
import {extractPlotObjects} from '../models/plot';
import {Result} from '../models/result';
import {getTransforms, hasWildcards} from '../models/shelf/spec';



export const selectBookmark = (state: State) => state.present.bookmark;
export const selectConfig = (state: State) => state.present.config;
export const selectData = (state: State) => state.present.dataset.data;
export const selectFilters = (state: State) => state.present.shelf.spec.filters;
export const selectShelf = (state: State) => state.present.shelf;
export const selectSchema = (state: State) => state.present.dataset.schema;
export const selectMainResult = (state: State) => state.present.result.main.modelGroup;

export const selectQuery = createSelector(
selectShelf,
Expand Down Expand Up @@ -67,3 +81,42 @@ export const selectSchemaFieldDefs = createSelector(
});
}
);

const selectMainResult = (state: State) => state.present.result.main;

const selectMainSpec = createSelector(
selectData, selectFilters, selectMainResult,
(data: Data, filters: Array<RangeFilter|OneOfFilter>, result: Result): FacetedCompositeUnitSpec => {
if (!result.modelGroup) {
return;
}
return {
data: data,
transform: getTransforms(filters),
...getTopSpecQueryItem(result.modelGroup).spec
};
}
);

export const selectMainResultForView = createSelector(
selectQuery, selectMainResult, selectMainSpec,
(query: Query, result: Result, spec: FacetedCompositeUnitSpec): {
spec: FacetedCompositeUnitSpec,
plots: PlotObject[]
} => {
if (result.isLoading || !result.modelGroup) {
return;
}
if (!hasWildcards(query.spec).hasAnyWildcard) {
return {
spec,
plots: null
};
} else {
return {
spec: null,
plots: extractPlotObjects(result.modelGroup)
};
}
}
);

0 comments on commit 7ef0578

Please sign in to comment.