Remove ImmutableJS, refactor search selectors (#48)

* Update prettier and wrap at 110 instead of 80

* Remove ImmutableJS from reducers

* Remove remaining ImmutableJS-oriented functionality

* Refactor search page mapStateToProps

* Add flow-types and tests for model/search

* Use spread operator instead of Object.assign

* Misc inline comments

* misc PR feedback

package.json

@@ -33,7 +33,6 @@
     "flow-bin": "^0.36.0",
     "fuzzy": "^0.1.1",
     "global": "^4.3.0",
-    "immutable": "^3.8.1",
     "is-promise": "^2.1.0",
     "isomorphic-fetch": "^2.2.1",
     "json-markup": "^1.0.0",
@@ -49,7 +48,6 @@
     "react-dom": "^15.5.0",
     "react-ga": "^2.1.2",
     "react-helmet": "^3.1.0",
-    "react-immutable-proptypes": "^2.1.0",
     "react-metrics": "^2.2.3",
     "react-redux": "^4.4.5",
     "react-router": "^2.7.0",

src/components/DependencyGraph/index.js

@@ -62,7 +62,6 @@ export default class DependencyGraphPage extends Component {
   render() {
     const { nodes, links, error, dependencies, loading } = this.props;
     const { graphType } = this.state;
-    const serviceCalls = dependencies.toJS();
     if (loading) {
       return (
         <div className="m1">
@@ -86,16 +85,17 @@ export default class DependencyGraphPage extends Component {
 
     const GRAPH_TYPE_OPTIONS = [{ type: 'FORCE_DIRECTED', name: 'Force Directed Graph' }];
 
-    if (serviceCalls.length <= 100) {
+    if (dependencies.length <= 100) {
       GRAPH_TYPE_OPTIONS.push({ type: 'DAG', name: 'DAG' });
     }
     return (
       <div className="my2">
         <Menu tabular>
           {GRAPH_TYPE_OPTIONS.map(option =>
             <Menu.Item
-              name={option.name}
               active={graphType === option.type}
+              key={option.type}
+              name={option.name}
               onClick={() => this.handleGraphTypeChange(option.type)}
             />
           )}
@@ -112,7 +112,7 @@ export default class DependencyGraphPage extends Component {
           }}
         >
           {graphType === 'FORCE_DIRECTED' && <DependencyForceGraph nodes={nodes} links={links} />}
-          {graphType === 'DAG' && <DAG serviceCalls={serviceCalls} />}
+          {graphType === 'DAG' && <DAG serviceCalls={dependencies} />}
         </div>
       </div>
     );
@@ -121,17 +121,14 @@ export default class DependencyGraphPage extends Component {
 
 // export connected component separately
 function mapStateToProps(state) {
-  const dependencies = state.dependencies.get('dependencies');
-  let nodes;
+  const { dependencies, error, loading } = state.dependencies;
   let links;
-  if (dependencies && dependencies.size > 0) {
-    const nodesAndLinks = formatDependenciesAsNodesAndLinks({ dependencies });
-    nodes = nodesAndLinks.nodes;
-    links = nodesAndLinks.links;
+  let nodes;
+  if (dependencies && dependencies.length > 0) {
+    const formatted = formatDependenciesAsNodesAndLinks({ dependencies });
+    links = formatted.links;
+    nodes = formatted.nodes;
   }
-  const error = state.dependencies.get('error');
-  const loading = state.dependencies.get('loading');
-
   return { loading, error, nodes, links, dependencies };
 }
 

src/components/SearchTracePage/TraceSearchForm.js

@@ -92,7 +92,7 @@ export function TraceSearchFormComponent(props) {
             name="service"
             component={SearchDropdownInput}
             className="ui dropdown"
-            items={services.concat({ name: '-' }).map(s => ({ text: s.name, value: s.name }))}
+            items={services.concat({ name: '-' }).map(s => ({ text: s.name, value: s.name, key: s.name }))}
           />
         </div>
 
@@ -102,7 +102,7 @@ export function TraceSearchFormComponent(props) {
               name="operation"
               component={SearchDropdownInput}
               className="ui dropdown"
-              items={operationsForService.concat('all').map(op => ({ text: op, value: op }))}
+              items={operationsForService.concat('all').map(op => ({ text: op, value: op, key: op }))}
             />
           </div>}
 
@@ -190,7 +190,12 @@ export function TraceSearchFormComponent(props) {
 TraceSearchFormComponent.propTypes = {
   handleSubmit: PropTypes.func,
   submitting: PropTypes.bool,
-  services: PropTypes.arrayOf(PropTypes.string),
+  services: PropTypes.arrayOf(
+    PropTypes.shape({
+      name: PropTypes.string,
+      operations: PropTypes.arrayOf(PropTypes.string),
+    })
+  ),
   selectedService: PropTypes.string,
   selectedLookback: PropTypes.string,
 };

src/components/SearchTracePage/TraceSearchResult.js

@@ -55,15 +55,15 @@ export default function TraceSearchResult({ trace, durationPercent = 100 }) {
             <span className="trace-search-result--spans">
               {numberOfSpans} span{numberOfSpans > 1 && 's'}
             </span>
-            {numberOfErredSpans &&
+            {Boolean(numberOfErredSpans) &&
               <span className="trace-search-result--erred-spans">
                 {numberOfErredSpans} error{numberOfErredSpans > 1 && 's'}
               </span>}
           </div>
           <div className="col col-6">
             {sortBy(services, s => s.name).map(service =>
               <div key={service.name} className="inline-block mr1 mb1">
-                <TraceServiceTag key={service.name} service={service} />
+                <TraceServiceTag service={service} />
               </div>
             )}
           </div>

src/components/SearchTracePage/TraceSearchResult.test.js

@@ -29,7 +29,7 @@ const testTraceProps = {
   services: [
     {
       name: 'Service A',
-      numberOfApperancesInTrace: 2,
+      numberOfSpans: 2,
       percentOfTrace: 50,
     },
   ],

src/components/SearchTracePage/TraceServiceTag.js

@@ -23,17 +23,17 @@ import React from 'react';
 import colorGenerator from '../../utils/color-generator';
 
 export default function TraceServiceTag({ service }) {
-  const { name, numberOfApperancesInTrace } = service;
+  const { name, numberOfSpans } = service;
   return (
     <div className="ui mini label" style={{ borderLeft: `5px solid ${colorGenerator.getColorByKey(name)}` }}>
-      {name} ({numberOfApperancesInTrace})
+      {name} ({numberOfSpans})
     </div>
   );
 }
 
 TraceServiceTag.propTypes = {
   service: PropTypes.shape({
     name: PropTypes.string.isRequired,
-    numberOfApperancesInTrace: PropTypes.number.isRequired,
+    numberOfSpans: PropTypes.number.isRequired,
   }).isRequired,
 };

src/components/SearchTracePage/TraceServiceTag.test.js

@@ -28,7 +28,7 @@ it('<SearchTracePage /> tests', () => {
     <TraceServiceTag
       service={{
         name: 'Service A',
-        numberOfApperancesInTrace: 1,
+        numberOfSpans: 1,
       }}
     />
   );

src/components/SearchTracePage/index.js

@@ -18,29 +18,23 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.
 
+import _values from 'lodash/values';
 import PropTypes from 'prop-types';
 import React, { Component } from 'react';
 import { bindActionCreators } from 'redux';
 import { connect } from 'react-redux';
 import { Field, reduxForm, formValueSelector } from 'redux-form';
 import { Link } from 'react-router';
 import { Sticky } from 'react-sticky';
-import * as jaegerApiActions from '../../actions/jaeger-api';
 
 import JaegerLogo from '../../img/jaeger-logo.svg';
 
+import * as jaegerApiActions from '../../actions/jaeger-api';
 import TraceSearchForm from './TraceSearchForm';
 import TraceSearchResult from './TraceSearchResult';
 import TraceResultsScatterPlot from './TraceResultsScatterPlot';
-import {
-  transformTraceResultsSelector,
-  getSortedTraceResults,
-  LONGEST_FIRST,
-  SHORTEST_FIRST,
-  MOST_SPANS,
-  LEAST_SPANS,
-  MOST_RECENT,
-} from '../../selectors/search';
+import * as orderBy from '../../model/order-by';
+import { sortTraces, getTraceSummaries } from '../../model/search';
 import { getPercentageOfDuration } from '../../utils/date';
 import getLastXformCacher from '../../utils/get-last-xform-cacher';
 
@@ -52,18 +46,18 @@ let TraceResultsFilterForm = () =>
     <div className="field inline">
       <label htmlFor="traceResultsSortBy">Sort</label>
       <Field name="sortBy" id="traceResultsSortBy" className="ui dropdown" component="select">
-        <option value={MOST_RECENT}>Most Recent</option>
-        <option value={LONGEST_FIRST}>Longest First</option>
-        <option value={SHORTEST_FIRST}>Shortest First</option>
-        <option value={MOST_SPANS}>Most Spans</option>
-        <option value={LEAST_SPANS}>Least Spans</option>
+        <option value={orderBy.MOST_RECENT}>Most Recent</option>
+        <option value={orderBy.LONGEST_FIRST}>Longest First</option>
+        <option value={orderBy.SHORTEST_FIRST}>Shortest First</option>
+        <option value={orderBy.MOST_SPANS}>Most Spans</option>
+        <option value={orderBy.LEAST_SPANS}>Least Spans</option>
       </Field>
     </div>
   </div>;
 TraceResultsFilterForm = reduxForm({
   form: 'traceResultsFilters',
   initialValues: {
-    sortBy: MOST_RECENT,
+    sortBy: orderBy.MOST_RECENT,
   },
 })(TraceResultsFilterForm);
 const traceResultsFiltersFormSelector = formValueSelector('traceResultsFilters');
@@ -194,13 +188,13 @@ SearchTracePage.propTypes = {
 };
 
 const stateTraceXformer = getLastXformCacher(stateTrace => {
-  const { traces: traceMap, loading, error: traceError } = stateTrace.toJS();
-  const traces = Object.keys(traceMap).map(traceID => traceMap[traceID]);
-  return { tracesSrc: { traces }, loading, traceError };
+  const { traces: traceMap, loading, error: traceError } = stateTrace;
+  const { traces, maxDuration } = getTraceSummaries(_values(traceMap));
+  return { traces, maxDuration, loading, traceError };
 });
 
 const stateServicesXformer = getLastXformCacher(stateServices => {
-  const { services: serviceList, operationsForService: opsBySvc, error: serviceError } = stateServices.toJS();
+  const { services: serviceList, operationsForService: opsBySvc, error: serviceError } = stateServices;
   const services = serviceList.map(name => ({
     name,
     operations: opsBySvc[name] || [],
@@ -211,18 +205,17 @@ const stateServicesXformer = getLastXformCacher(stateServices => {
 function mapStateToProps(state) {
   const query = state.routing.locationBeforeTransitions.query;
   const isHomepage = !Object.keys(query).length;
-  const { tracesSrc, loading, traceError } = stateTraceXformer(state.trace);
-  const { traces, maxDuration } = transformTraceResultsSelector(tracesSrc);
+  const { traces, maxDuration, loading, traceError } = stateTraceXformer(state.trace);
   const { services, serviceError } = stateServicesXformer(state.services);
-  const sortBy = traceResultsFiltersFormSelector(state, 'sortBy');
-  const traceResultsSorted = getSortedTraceResults(traces, sortBy);
   const errorMessage = serviceError || traceError ? `${serviceError || ''} ${traceError || ''}` : '';
+  const sortBy = traceResultsFiltersFormSelector(state, 'sortBy');
+  sortTraces(traces, sortBy);
 
   return {
     isHomepage,
     sortTracesBy: sortBy,
-    traceResults: traceResultsSorted,
-    numberOfTraceResults: traceResultsSorted.length,
+    traceResults: traces,
+    numberOfTraceResults: traces.length,
     maxTraceDuration: maxDuration,
     urlQueryParams: query,
     services,
@@ -233,7 +226,6 @@ function mapStateToProps(state) {
 
 function mapDispatchToProps(dispatch) {
   const { searchTraces, fetchServices } = bindActionCreators(jaegerApiActions, dispatch);
-
   return {
     searchTraces,
     fetchServices,

src/components/TracePage/TraceTimelineViewer/SpanDetail.js

@@ -44,7 +44,7 @@ function CollapsePanel(props) {
   );
 }
 CollapsePanel.propTypes = {
-  header: PropTypes.element.isRequired,
+  header: PropTypes.node.isRequired,
   onToggleOpen: PropTypes.func.isRequired,
   children: PropTypes.element.isRequired,
   open: PropTypes.bool.isRequired,

src/components/TracePage/index.js

@@ -168,18 +168,13 @@ export default class TracePage extends Component {
 
 // export connected component separately
 function mapStateToProps(state, ownProps) {
-  const { params: { id } } = ownProps;
-
-  let trace = state.trace.getIn(['traces', id]);
+  const { id } = ownProps.params;
+  let trace = state.trace.traces[id];
   if (trace && !(trace instanceof Error)) {
-    trace = trace.toJS();
     trace = dropEmptyStartTimeSpans(trace);
     trace = hydrateSpansWithProcesses(trace);
   }
-
-  const loading = state.trace.get('loading');
-
-  return { id, loading, trace };
+  return { id, trace, loading: state.trace.loading };
 }
 
 function mapDispatchToProps(dispatch) {

src/reducers/index.test.js → src/model/order-by.js

@@ -18,9 +18,8 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.
 
-import jaegerReducers from './index';
-import traceReducer from './trace';
-
-it('jaegerReducers should contain the trace reducer', () => {
-  expect(jaegerReducers.trace).toBe(traceReducer);
-});
+export const MOST_RECENT = 'MOST_RECENT';
+export const LONGEST_FIRST = 'LONGEST_FIRST';
+export const SHORTEST_FIRST = 'SHORTEST_FIRST';
+export const MOST_SPANS = 'MOST_SPANS';
+export const LEAST_SPANS = 'LEAST_SPANS';