Improve Quick Open performance (firefox-devtools#8209)

ruturajv · Apr 29, 2019 · 0c2a1ba · 0c2a1ba
1 parent e5dfa84
commit 0c2a1ba
Show file tree

Hide file tree

Showing 6 changed files with 96 additions and 50 deletions.
diff --git a/src/components/QuickOpenModal.js b/src/components/QuickOpenModal.js
@@ -7,6 +7,7 @@ import React, { Component } from "react";
 import { connect } from "../utils/connect";
 import fuzzyAldrin from "fuzzaldrin-plus";
 import { basename } from "../utils/path";
+import { throttle } from "lodash";
 
 import actions from "../actions";
 import {
@@ -43,8 +44,8 @@ import "./QuickOpenModal.css";
 
 type Props = {
   enabled: boolean,
-  sources: Array<Object>,
   selectedSource?: Source,
+  displayedSources: Source[],
   query: string,
   searchType: QuickOpenType,
   symbols: FormattedSymbolDeclarations,
@@ -69,12 +70,16 @@ type GotoLocationType = {
   column?: number
 };
 
+const updateResultsThrottle = 100;
 const maxResults = 100;
 
 function filter(values, query) {
+  const preparedQuery = fuzzyAldrin.prepareQuery(query);
+
   return fuzzyAldrin.filter(values, query, {
     key: "value",
-    maxResults: maxResults
+    maxResults: maxResults,
+    preparedQuery
   });
 }
 
@@ -127,7 +132,9 @@ export class QuickOpenModal extends Component<Props, State> {
   };
 
   searchSources = (query: string) => {
-    const { sources } = this.props;
+    const { displayedSources, tabs } = this.props;
+    const tabUrls = new Set(tabs.map((tab: Tab) => tab.url));
+    const sources = formatSources(displayedSources, tabUrls);
     const results =
       query == "" ? sources : filter(sources, this.dropGoto(query));
     return this.setResults(results);
@@ -158,16 +165,24 @@ export class QuickOpenModal extends Component<Props, State> {
   };
 
   showTopSources = () => {
-    const { tabs, sources } = this.props;
+    const { displayedSources, tabs } = this.props;
+    const tabUrls = new Set(tabs.map((tab: Tab) => tab.url));
+
     if (tabs.length > 0) {
-      const tabUrls = tabs.map((tab: Tab) => tab.url);
-      this.setResults(sources.filter(source => tabUrls.includes(source.url)));
+      this.setResults(
+        formatSources(
+          displayedSources.filter(
+            source => !!source.url && tabUrls.has(source.url)
+          ),
+          tabUrls
+        )
+      );
     } else {
-      this.setResults(sources);
+      this.setResults(formatSources(displayedSources, tabUrls));
     }
   };
 
-  updateResults = (query: string) => {
+  updateResults = throttle((query: string) => {
     if (this.isGotoQuery()) {
       return;
     }
@@ -185,7 +200,7 @@ export class QuickOpenModal extends Component<Props, State> {
     }
 
     return this.searchSources(query);
-  };
+  }, updateResultsThrottle);
 
   setModifier = (item: QuickOpenResult) => {
     if (["@", "#", ":"].includes(item.id)) {
@@ -422,16 +437,17 @@ export class QuickOpenModal extends Component<Props, State> {
 /* istanbul ignore next: ignoring testing of redux connection stuff */
 function mapStateToProps(state) {
   const selectedSource = getSelectedSource(state);
+  const tabs = getTabs(state);
 
   return {
     enabled: getQuickOpenEnabled(state),
-    sources: formatSources(getDisplayedSourcesList(state), getTabs(state)),
     selectedSource,
+    displayedSources: getDisplayedSourcesList(state),
     symbols: formatSymbols(getSymbols(state, selectedSource)),
     symbolsLoading: isSymbolsLoading(state, selectedSource),
     query: getQuickOpenQuery(state),
     searchType: getQuickOpenType(state),
-    tabs: getTabs(state)
+    tabs
   };
 }
 

diff --git a/src/components/test/QuickOpenModal.spec.js b/src/components/test/QuickOpenModal.spec.js
@@ -7,9 +7,14 @@
 
 import React from "react";
 import { shallow, mount } from "enzyme";
+import lodash from "lodash";
 import { QuickOpenModal } from "../QuickOpenModal";
 
 jest.mock("fuzzaldrin-plus");
+jest.unmock("lodash");
+
+// $FlowIgnore
+lodash.throttle = jest.fn(fn => fn);
 
 import { filter } from "fuzzaldrin-plus";
 
@@ -18,7 +23,7 @@ function generateModal(propOverrides, renderType = "shallow") {
     enabled: false,
     query: "",
     searchType: "sources",
-    sources: [],
+    displayedSources: [],
     tabs: [],
     selectSpecificLocation: jest.fn(),
     setQuickOpenQuery: jest.fn(),
@@ -111,12 +116,24 @@ describe("QuickOpenModal", () => {
       {
         enabled: true,
         query: "",
-        sources: [{ url: "mozilla.com" }],
+        displayedSources: [
+          // $FlowIgnore
+          { url: "mozilla.com", relativeUrl: true }
+        ],
         tabs: [generateTab("mozilla.com")]
       },
       "shallow"
     );
-    expect(wrapper.state("results")).toEqual([{ url: "mozilla.com" }]);
+    expect(wrapper.state("results")).toEqual([
+      {
+        id: undefined,
+        icon: "tab result-item-icon",
+        subtitle: "true",
+        title: "mozilla.com",
+        url: "mozilla.com",
+        value: "true"
+      }
+    ]);
   });
 
   describe("shows loading", () => {
@@ -786,6 +803,7 @@ describe("QuickOpenModal", () => {
         },
         "mount"
       );
+
       expect(wrapper).toMatchSnapshot();
     });
 

diff --git a/src/components/test/__snapshots__/QuickOpenModal.spec.js.snap b/src/components/test/__snapshots__/QuickOpenModal.spec.js.snap
@@ -4,6 +4,7 @@ exports[`QuickOpenModal Basic render with mount & searchType = functions 1`] = `
 <QuickOpenModal
   clearHighlightLineRange={[MockFunction]}
   closeQuickOpen={[MockFunction]}
+  displayedSources={Array []}
   enabled={true}
   highlightLineRange={[MockFunction]}
   isOriginal={false}
@@ -12,7 +13,6 @@ exports[`QuickOpenModal Basic render with mount & searchType = functions 1`] = `
   selectSpecificLocation={[MockFunction]}
   setQuickOpenQuery={[MockFunction]}
   shortcutsModalEnabled={false}
-  sources={Array []}
   symbols={
     Object {
       "functions": Array [],
@@ -130,6 +130,7 @@ exports[`QuickOpenModal Basic render with mount & searchType = variables 1`] = `
 <QuickOpenModal
   clearHighlightLineRange={[MockFunction]}
   closeQuickOpen={[MockFunction]}
+  displayedSources={Array []}
   enabled={true}
   highlightLineRange={[MockFunction]}
   isOriginal={false}
@@ -138,7 +139,6 @@ exports[`QuickOpenModal Basic render with mount & searchType = variables 1`] = `
   selectSpecificLocation={[MockFunction]}
   setQuickOpenQuery={[MockFunction]}
   shortcutsModalEnabled={false}
-  sources={Array []}
   symbols={
     Object {
       "functions": Array [],
@@ -240,6 +240,7 @@ exports[`QuickOpenModal Basic render with mount 1`] = `
 <QuickOpenModal
   clearHighlightLineRange={[MockFunction]}
   closeQuickOpen={[MockFunction]}
+  displayedSources={Array []}
   enabled={true}
   highlightLineRange={[MockFunction]}
   isOriginal={false}
@@ -248,7 +249,6 @@ exports[`QuickOpenModal Basic render with mount 1`] = `
   selectSpecificLocation={[MockFunction]}
   setQuickOpenQuery={[MockFunction]}
   shortcutsModalEnabled={false}
-  sources={Array []}
   symbols={
     Object {
       "functions": Array [],
@@ -400,6 +400,7 @@ exports[`QuickOpenModal Simple goto search query = :abc & searchType = goto 1`]
 <QuickOpenModal
   clearHighlightLineRange={[MockFunction]}
   closeQuickOpen={[MockFunction]}
+  displayedSources={Array []}
   enabled={true}
   highlightLineRange={[MockFunction]}
   isOriginal={false}
@@ -408,7 +409,6 @@ exports[`QuickOpenModal Simple goto search query = :abc & searchType = goto 1`]
   selectSpecificLocation={[MockFunction]}
   setQuickOpenQuery={[MockFunction]}
   shortcutsModalEnabled={false}
-  sources={Array []}
   symbols={
     Object {
       "functions": Array [],
@@ -515,6 +515,7 @@ exports[`QuickOpenModal showErrorEmoji false when count + query 1`] = `
 <QuickOpenModal
   clearHighlightLineRange={[MockFunction]}
   closeQuickOpen={[MockFunction]}
+  displayedSources={Array []}
   enabled={true}
   highlightLineRange={[MockFunction]}
   isOriginal={false}
@@ -523,7 +524,6 @@ exports[`QuickOpenModal showErrorEmoji false when count + query 1`] = `
   selectSpecificLocation={[MockFunction]}
   setQuickOpenQuery={[MockFunction]}
   shortcutsModalEnabled={false}
-  sources={Array []}
   symbols={
     Object {
       "functions": Array [],
@@ -672,6 +672,7 @@ exports[`QuickOpenModal showErrorEmoji false when goto numeric ':2222' 1`] = `
 <QuickOpenModal
   clearHighlightLineRange={[MockFunction]}
   closeQuickOpen={[MockFunction]}
+  displayedSources={Array []}
   enabled={true}
   highlightLineRange={[MockFunction]}
   isOriginal={false}
@@ -680,7 +681,6 @@ exports[`QuickOpenModal showErrorEmoji false when goto numeric ':2222' 1`] = `
   selectSpecificLocation={[MockFunction]}
   setQuickOpenQuery={[MockFunction]}
   shortcutsModalEnabled={false}
-  sources={Array []}
   symbols={
     Object {
       "functions": Array [],
@@ -786,6 +786,7 @@ exports[`QuickOpenModal showErrorEmoji false when no query 1`] = `
 <QuickOpenModal
   clearHighlightLineRange={[MockFunction]}
   closeQuickOpen={[MockFunction]}
+  displayedSources={Array []}
   enabled={true}
   highlightLineRange={[MockFunction]}
   isOriginal={false}
@@ -794,7 +795,6 @@ exports[`QuickOpenModal showErrorEmoji false when no query 1`] = `
   selectSpecificLocation={[MockFunction]}
   setQuickOpenQuery={[MockFunction]}
   shortcutsModalEnabled={false}
-  sources={Array []}
   symbols={
     Object {
       "functions": Array [],
@@ -911,6 +911,7 @@ exports[`QuickOpenModal showErrorEmoji true when goto not numeric ':22k22' 1`] =
 <QuickOpenModal
   clearHighlightLineRange={[MockFunction]}
   closeQuickOpen={[MockFunction]}
+  displayedSources={Array []}
   enabled={true}
   highlightLineRange={[MockFunction]}
   isOriginal={false}
@@ -919,7 +920,6 @@ exports[`QuickOpenModal showErrorEmoji true when goto not numeric ':22k22' 1`] =
   selectSpecificLocation={[MockFunction]}
   setQuickOpenQuery={[MockFunction]}
   shortcutsModalEnabled={false}
-  sources={Array []}
   symbols={
     Object {
       "functions": Array [],
@@ -1025,6 +1025,7 @@ exports[`QuickOpenModal showErrorEmoji true when no count + query 1`] = `
 <QuickOpenModal
   clearHighlightLineRange={[MockFunction]}
   closeQuickOpen={[MockFunction]}
+  displayedSources={Array []}
   enabled={true}
   highlightLineRange={[MockFunction]}
   isOriginal={false}
@@ -1033,7 +1034,6 @@ exports[`QuickOpenModal showErrorEmoji true when no count + query 1`] = `
   selectSpecificLocation={[MockFunction]}
   setQuickOpenQuery={[MockFunction]}
   shortcutsModalEnabled={false}
-  sources={Array []}
   symbols={
     Object {
       "functions": Array [],
@@ -1167,6 +1167,7 @@ exports[`QuickOpenModal updateResults on enable 1`] = `
 <QuickOpenModal
   clearHighlightLineRange={[MockFunction]}
   closeQuickOpen={[MockFunction]}
+  displayedSources={Array []}
   enabled={false}
   highlightLineRange={[MockFunction]}
   isOriginal={false}
@@ -1175,7 +1176,6 @@ exports[`QuickOpenModal updateResults on enable 1`] = `
   selectSpecificLocation={[MockFunction]}
   setQuickOpenQuery={[MockFunction]}
   shortcutsModalEnabled={false}
-  sources={Array []}
   symbols={
     Object {
       "functions": Array [],
@@ -1192,6 +1192,7 @@ exports[`QuickOpenModal updateResults on enable 2`] = `
 <QuickOpenModal
   clearHighlightLineRange={[MockFunction]}
   closeQuickOpen={[MockFunction]}
+  displayedSources={Array []}
   enabled={true}
   highlightLineRange={[MockFunction]}
   isOriginal={false}
@@ -1200,7 +1201,6 @@ exports[`QuickOpenModal updateResults on enable 2`] = `
   selectSpecificLocation={[MockFunction]}
   setQuickOpenQuery={[MockFunction]}
   shortcutsModalEnabled={false}
-  sources={Array []}
   symbols={
     Object {
       "functions": Array [],

diff --git a/src/utils/quick-open.js b/src/utils/quick-open.js
@@ -14,7 +14,7 @@ import {
 import type { Location as BabelLocation } from "@babel/types";
 import type { Symbols } from "../reducers/ast";
 import type { QuickOpenType } from "../reducers/quick-open";
-import type { TabList } from "../reducers/tabs";
+import type { Tab } from "../reducers/tabs";
 import type { Source } from "../types";
 import type {
   SymbolDeclaration,
@@ -58,7 +58,10 @@ export function parseLineColumn(query: string) {
   }
 }
 
-export function formatSourcesForList(source: Source, tabs: TabList) {
+export function formatSourcesForList(
+  source: Source,
+  tabUrls: Set<$PropertyType<Tab, "url">>
+) {
   const title = getFilename(source);
   const relativeUrlWithQuery = `${source.relativeUrl}${getSourceQueryString(
     source
@@ -68,7 +71,7 @@ export function formatSourcesForList(source: Source, tabs: TabList) {
     value: relativeUrlWithQuery,
     title,
     subtitle: relativeUrlWithQuery,
-    icon: tabs.some(tab => tab.url == source.url)
+    icon: tabUrls.has(source.url)
       ? "tab result-item-icon"
       : classnames(getSourceClassnames(source), "result-item-icon"),
     id: source.id,
@@ -136,10 +139,9 @@ export function formatShortcutResults(): Array<QuickOpenResult> {
 
 export function formatSources(
   sources: Source[],
-  tabs: TabList
+  tabUrls: Set<$PropertyType<Tab, "url">>
 ): Array<QuickOpenResult> {
   return sources
-    .filter(source => !isPretty(source))
-    .filter(({ relativeUrl }) => !!relativeUrl)
-    .map(source => formatSourcesForList(source, tabs));
+    .filter(source => !!source.relativeUrl && !isPretty(source))
+    .map(source => formatSourcesForList(source, tabUrls));
 }