Browse Source

Prevent Explore from updating when typing query

- remove `edited` property of query objects
- query object semantics changes to be initial values only
- resetting or externally modifying a query is done via giving it a new
  react key, which replaces the row completely
- query changes while typing are sent up to Explore to be saved in a
  local property that is not part of the component state (prevent
  control circle)
- all Explore functions using the queries must access the local property
  for current values
David Kaltschmidt 7 years ago
parent
commit
bdae399322

+ 94 - 71
public/app/features/explore/Explore.tsx

@@ -2,7 +2,7 @@ import React from 'react';
 import { hot } from 'react-hot-loader';
 import Select from 'react-select';
 
-import { ExploreState, ExploreUrlState } from 'app/types/explore';
+import { ExploreState, ExploreUrlState, Query } from 'app/types/explore';
 import kbn from 'app/core/utils/kbn';
 import colors from 'app/core/utils/colors';
 import store from 'app/core/store';
@@ -61,38 +61,50 @@ interface ExploreProps {
 
 export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
   el: any;
+  /**
+   * Current query expressions of the rows including their modifications, used for running queries.
+   * Not kept in component state to prevent edit-render roundtrips.
+   */
+  queryExpressions: string[];
 
   constructor(props) {
     super(props);
-    // Split state overrides everything
     const splitState: ExploreState = props.splitState;
-    const { datasource, queries, range } = props.urlState;
-    this.state = {
-      datasource: null,
-      datasourceError: null,
-      datasourceLoading: null,
-      datasourceMissing: false,
-      datasourceName: datasource,
-      exploreDatasources: [],
-      graphResult: null,
-      history: [],
-      latency: 0,
-      loading: false,
-      logsResult: null,
-      queries: ensureQueries(queries),
-      queryErrors: [],
-      queryHints: [],
-      range: range || { ...DEFAULT_RANGE },
-      requestOptions: null,
-      showingGraph: true,
-      showingLogs: true,
-      showingTable: true,
-      supportsGraph: null,
-      supportsLogs: null,
-      supportsTable: null,
-      tableResult: null,
-      ...splitState,
-    };
+    let initialQueries: Query[];
+    if (splitState) {
+      // Split state overrides everything
+      this.state = splitState;
+      initialQueries = splitState.queries;
+    } else {
+      const { datasource, queries, range } = props.urlState as ExploreUrlState;
+      initialQueries = ensureQueries(queries);
+      this.state = {
+        datasource: null,
+        datasourceError: null,
+        datasourceLoading: null,
+        datasourceMissing: false,
+        datasourceName: datasource,
+        exploreDatasources: [],
+        graphResult: null,
+        history: [],
+        latency: 0,
+        loading: false,
+        logsResult: null,
+        queries: initialQueries,
+        queryErrors: [],
+        queryHints: [],
+        range: range || { ...DEFAULT_RANGE },
+        requestOptions: null,
+        showingGraph: true,
+        showingLogs: true,
+        showingTable: true,
+        supportsGraph: null,
+        supportsLogs: null,
+        supportsTable: null,
+        tableResult: null,
+      };
+    }
+    this.queryExpressions = initialQueries.map(q => q.query);
   }
 
   async componentDidMount() {
@@ -152,9 +164,10 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
     }
 
     // Keep queries but reset edit state
-    const nextQueries = this.state.queries.map(q => ({
+    const nextQueries = this.state.queries.map((q, i) => ({
       ...q,
-      edited: false,
+      key: generateQueryKey(i),
+      query: this.queryExpressions[i],
     }));
 
     this.setState(
@@ -183,6 +196,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
 
   onAddQueryRow = index => {
     const { queries } = this.state;
+    this.queryExpressions[index + 1] = '';
     const nextQueries = [
       ...queries.slice(0, index + 1),
       { query: '', key: generateQueryKey() },
@@ -209,29 +223,28 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
   };
 
   onChangeQuery = (value: string, index: number, override?: boolean) => {
-    const { queries } = this.state;
-    let { queryErrors, queryHints } = this.state;
-    const prevQuery = queries[index];
-    const edited = override ? false : prevQuery.query !== value;
-    const nextQuery = {
-      ...queries[index],
-      edited,
-      query: value,
-    };
-    const nextQueries = [...queries];
-    nextQueries[index] = nextQuery;
+    // Keep current value in local cache
+    this.queryExpressions[index] = value;
+
+    // Replace query row on override
     if (override) {
-      queryErrors = [];
-      queryHints = [];
+      const { queries } = this.state;
+      const nextQuery: Query = {
+        key: generateQueryKey(index),
+        query: value,
+      };
+      const nextQueries = [...queries];
+      nextQueries[index] = nextQuery;
+
+      this.setState(
+        {
+          queryErrors: [],
+          queryHints: [],
+          queries: nextQueries,
+        },
+        this.onSubmit
+      );
     }
-    this.setState(
-      {
-        queryErrors,
-        queryHints,
-        queries: nextQueries,
-      },
-      override ? () => this.onSubmit() : undefined
-    );
   };
 
   onChangeTime = nextRange => {
@@ -243,6 +256,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
   };
 
   onClickClear = () => {
+    this.queryExpressions = [''];
     this.setState(
       {
         graphResult: null,
@@ -275,9 +289,8 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
 
   onClickSplit = () => {
     const { onChangeSplit } = this.props;
-    const state = { ...this.state };
-    state.queries = state.queries.map(({ edited, ...rest }) => rest);
     if (onChangeSplit) {
+      const state = this.cloneState();
       onChangeSplit(true, state);
       this.saveState();
     }
@@ -297,23 +310,22 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
       let nextQueries;
       if (index === undefined) {
         // Modify all queries
-        nextQueries = queries.map(q => ({
-          ...q,
-          edited: false,
-          query: datasource.modifyQuery(q.query, action),
+        nextQueries = queries.map((q, i) => ({
+          key: generateQueryKey(i),
+          query: datasource.modifyQuery(this.queryExpressions[i], action),
         }));
       } else {
         // Modify query only at index
         nextQueries = [
           ...queries.slice(0, index),
           {
-            ...queries[index],
-            edited: false,
-            query: datasource.modifyQuery(queries[index].query, action),
+            key: generateQueryKey(index),
+            query: datasource.modifyQuery(this.queryExpressions[index], action),
           },
           ...queries.slice(index + 1),
         ];
       }
+      this.queryExpressions = nextQueries.map(q => q.query);
       this.setState({ queries: nextQueries }, () => this.onSubmit());
     }
   };
@@ -324,6 +336,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
       return;
     }
     const nextQueries = [...queries.slice(0, index), ...queries.slice(index + 1)];
+    this.queryExpressions = nextQueries.map(q => q.query);
     this.setState({ queries: nextQueries }, () => this.onSubmit());
   };
 
@@ -341,7 +354,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
     this.saveState();
   };
 
-  onQuerySuccess(datasourceId: string, queries: any[]): void {
+  onQuerySuccess(datasourceId: string, queries: string[]): void {
     // save queries to history
     let { history } = this.state;
     const { datasource } = this.state;
@@ -352,8 +365,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
     }
 
     const ts = Date.now();
-    queries.forEach(q => {
-      const { query } = q;
+    queries.forEach(query => {
       history = [{ query, ts }, ...history];
     });
 
@@ -368,16 +380,16 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
   }
 
   buildQueryOptions(targetOptions: { format: string; hinting?: boolean; instant?: boolean }) {
-    const { datasource, queries, range } = this.state;
+    const { datasource, range } = this.state;
     const resolution = this.el.offsetWidth;
     const absoluteRange = {
       from: parseDate(range.from, false),
       to: parseDate(range.to, true),
     };
     const { interval } = kbn.calculateInterval(absoluteRange, resolution, datasource.interval);
-    const targets = queries.map(q => ({
+    const targets = this.queryExpressions.map(q => ({
       ...targetOptions,
-      expr: q.query,
+      expr: q,
     }));
     return {
       interval,
@@ -387,7 +399,8 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
   }
 
   async runGraphQuery() {
-    const { datasource, queries } = this.state;
+    const { datasource } = this.state;
+    const queries = [...this.queryExpressions];
     if (!hasQuery(queries)) {
       return;
     }
@@ -409,7 +422,8 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
   }
 
   async runTableQuery() {
-    const { datasource, queries } = this.state;
+    const queries = [...this.queryExpressions];
+    const { datasource } = this.state;
     if (!hasQuery(queries)) {
       return;
     }
@@ -433,7 +447,8 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
   }
 
   async runLogsQuery() {
-    const { datasource, queries } = this.state;
+    const queries = [...this.queryExpressions];
+    const { datasource } = this.state;
     if (!hasQuery(queries)) {
       return;
     }
@@ -461,9 +476,17 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
     return datasource.metadataRequest(url);
   };
 
+  cloneState(): ExploreState {
+    // Copy state, but copy queries including modifications
+    return {
+      ...this.state,
+      queries: ensureQueries(this.queryExpressions.map(query => ({ query }))),
+    };
+  }
+
   saveState = () => {
     const { stateKey, onSaveState } = this.props;
-    onSaveState(stateKey, this.state);
+    onSaveState(stateKey, this.cloneState());
   };
 
   render() {

+ 3 - 4
public/app/features/explore/QueryRows.tsx

@@ -44,14 +44,14 @@ class QueryRow extends PureComponent<any, {}> {
   };
 
   render() {
-    const { edited, history, query, queryError, queryHint, request, supportsLogs } = this.props;
+    const { history, query, queryError, queryHint, request, supportsLogs } = this.props;
     return (
       <div className="query-row">
         <div className="query-row-field">
           <QueryField
             error={queryError}
             hint={queryHint}
-            initialQuery={edited ? null : query}
+            initialQuery={query}
             history={history}
             portalPrefix="explore"
             onClickHintFix={this.onClickHintFix}
@@ -79,7 +79,7 @@ class QueryRow extends PureComponent<any, {}> {
 
 export default class QueryRows extends PureComponent<any, {}> {
   render() {
-    const { className = '', queries, queryErrors = [], queryHints = [], ...handlers } = this.props;
+    const { className = '', queries, queryErrors, queryHints, ...handlers } = this.props;
     return (
       <div className={className}>
         {queries.map((q, index) => (
@@ -89,7 +89,6 @@ export default class QueryRows extends PureComponent<any, {}> {
             query={q.query}
             queryError={queryErrors[index]}
             queryHint={queryHints[index]}
-            edited={q.edited}
             {...handlers}
           />
         ))}

+ 6 - 4
public/app/features/explore/utils/query.ts

@@ -1,14 +1,16 @@
-export function generateQueryKey(index = 0) {
+import { Query } from 'app/types/explore';
+
+export function generateQueryKey(index = 0): string {
   return `Q-${Date.now()}-${Math.random()}-${index}`;
 }
 
-export function ensureQueries(queries?) {
+export function ensureQueries(queries?: Query[]): Query[] {
   if (queries && typeof queries === 'object' && queries.length > 0 && typeof queries[0].query === 'string') {
     return queries.map(({ query }, i) => ({ key: generateQueryKey(i), query }));
   }
   return [{ key: generateQueryKey(), query: '' }];
 }
 
-export function hasQuery(queries) {
-  return queries.some(q => q.query);
+export function hasQuery(queries: string[]): boolean {
+  return queries.some(q => Boolean(q));
 }

+ 11 - 1
public/app/types/explore.ts

@@ -10,7 +10,6 @@ export interface Range {
 
 export interface Query {
   query: string;
-  edited?: boolean;
   key?: string;
 }
 
@@ -26,8 +25,19 @@ export interface ExploreState {
   latency: number;
   loading: any;
   logsResult: any;
+  /**
+   * Initial rows of queries to push down the tree.
+   * Modifications do not end up here, but in `this.queryExpressions`.
+   * The only way to reset a query is to change its `key`.
+   */
   queries: Query[];
+  /**
+   * Errors caused by the running the query row.
+   */
   queryErrors: any[];
+  /**
+   * Hints gathered for the query row.
+   */
   queryHints: any[];
   range: Range;
   requestOptions: any;