fix: command palette (#263)

A regression (#262) which broke the command palette was introduced (but not published). This fixes that regression.

- The regression was introduced by @sqs's change to e-c-c here: sourcegraph/extensions-client-common@19df39f
- The dependency was updated in this repository by @chrismwendt here: 8616d66

The issue is that we are using a `@shopify/react-shortcuts` package to declare keyboard shortcuts in e-c-c. The package registers keyboard shortcuts via a `<Shortcut>` component which requires that a `<ShortcutProvider>` component be somewhere above it in the React rendering tree, in order to pass down some React context.

In the Sourcegraph web application, we provide such an implementation: https://sourcegraph.sgdev.org/github.com/sourcegraph/sourcegraph/-/blob/src/SourcegraphWebApp.tsx#L259:14 Here in the browser extension, however, we do not -- and this causes any `<Shortcut>` component to break due to missing React context data. If we were to publish the browser extension today, the Sourcegraph Extension command palette would be broken outright (clicking on it causes it to disappear).

Fixing the issue is tricky: `@shopify/react-shortcuts`'s `ShortcutProvider` component is designed with the assumption that there will only ever be a single React component rendering the entire application. It uses this assumption to only register a single document keyboard event handler, [otherwise it will install multiple global event handlers to the `document` that conflict with one another](https://github.com/Shopify/quilt/blob/master/packages/react-shortcuts/src/ShortcutManager/ShortcutManager.tsx#L21). In most applications, this assumption holds true, but in our situation here with the browser extension this is not true as we inject independent React views into several different locations on the page and multiple expect to be able to add `<Shortcut>`s.

Fixing this becomes more tricky because `@shopify/react-shortcuts` does not expose [the `Provider` part of the `React.createContext` result](https://github.com/Shopify/quilt/blob/master/packages/react-shortcuts/src/ShortcutProvider/ShortcutProvider.tsx#L12) -- meaning it is impossible for us to implement our own `ShortcutProvider` that fixes this issue. I've had to temporarily fork react-shortcuts to expose this, and if all looks good I will create a PR upstream for this: https://github.com/slimsag/quilt/tree/publish-provider In the meantime, we will have to use my fork everywhere (if we don't, React context will not grab the right data and it'll e.g. break the command palette on Sourcegraph.com).

This does not actually cause us to register shortcuts in the browser extension for the command palette -- but we certainly can now after this change.

See also:

- sourcegraph/extensions-client-common#60
- https://github.com/sourcegraph/sourcegraph/pull/500

Fixes #262

Author: Stephen Gutekanst

package.json

@@ -111,6 +111,7 @@
     "worker-loader": "^2.0.0"
   },
   "dependencies": {
+    "@slimsag/react-shortcuts": "^1.2.1",
     "@sourcegraph/codeintellify": "^3.9.0",
     "@sourcegraph/extensions-client-common": "^10.2.2",
     "@sourcegraph/react-loading-spinner": "0.0.6",

src/libs/code_intelligence/extensions.tsx

@@ -25,6 +25,7 @@ import * as H from 'history'
 import { isErrorLike } from '../../shared/backend/errors'
 import { createExtensionsContextController, createMessageTransports } from '../../shared/backend/extensions'
 import { GlobalDebug } from '../../shared/components/GlobalDebug'
+import { ShortcutProvider } from '../../shared/components/ShortcutProvider'
 import { sourcegraphUrl } from '../../shared/util/context'
 import { getGlobalDebugMount } from '../github/extensions'
 import { MountGetter } from './code_intelligence'
@@ -111,11 +112,13 @@ export function initializeExtensions(
     const { extensionsContextController, extensionsController } = createControllers(documents)
 
     render(
-        <CommandListPopoverButton
-            extensionsController={extensionsController}
-            menu={ContributableMenu.CommandPalette}
-            extensions={extensionsContextController}
-        />,
+        <ShortcutProvider>
+            <CommandListPopoverButton
+                extensionsController={extensionsController}
+                menu={ContributableMenu.CommandPalette}
+                extensions={extensionsContextController}
+            />
+        </ShortcutProvider>,
         getCommandPaletteMount()
     )
 

src/libs/github/extensions.tsx

@@ -8,6 +8,7 @@ import { ContributableMenu } from 'sourcegraph/module/protocol'
 import * as React from 'react'
 import { render } from 'react-dom'
 import { GlobalDebug } from '../../shared/components/GlobalDebug'
+import { ShortcutProvider } from '../../shared/components/ShortcutProvider'
 
 export function getCommandPaletteMount(): HTMLElement {
     const headerElem = document.querySelector('div.HeaderMenu>div:last-child')
@@ -51,11 +52,13 @@ export function injectExtensionsGlobalComponents({
     extensionsContextController: Controller<ConfigurationSubject, Settings>
 }): void {
     render(
-        <CommandListPopoverButton
-            extensionsController={extensionsController}
-            menu={ContributableMenu.CommandPalette}
-            extensions={extensionsContextController}
-        />,
+        <ShortcutProvider>
+            <CommandListPopoverButton
+                extensionsController={extensionsController}
+                menu={ContributableMenu.CommandPalette}
+                extensions={extensionsContextController}
+            />
+        </ShortcutProvider>,
         getCommandPaletteMount()
     )
 

src/shared/components/GlobalDebug.tsx

@@ -6,6 +6,7 @@ import * as H from 'history'
 import MenuDownIcon from 'mdi-react/MenuDownIcon'
 import * as React from 'react'
 import { sourcegraphUrl } from '../util/context'
+import { ShortcutProvider } from './ShortcutProvider'
 
 interface Props {
     location: H.Location
@@ -28,13 +29,17 @@ export const GlobalDebug: React.SFC<Props> = props =>
         <div className="global-debug navbar navbar-expand">
             <ul className="navbar-nav align-items-center">
                 <li className="nav-item">
-                    <ExtensionStatusPopover
-                        location={props.location}
-                        loaderIcon={LoadingSpinner as React.ComponentType<{ className: string; onClick?: () => void }>}
-                        caretIcon={MenuDownIcon as React.ComponentType<{ className: string; onClick?: () => void }>}
-                        extensionsController={props.extensionsController}
-                        link={ExtensionLink}
-                    />
+                    <ShortcutProvider>
+                        <ExtensionStatusPopover
+                            location={props.location}
+                            loaderIcon={
+                                LoadingSpinner as React.ComponentType<{ className: string; onClick?: () => void }>
+                            }
+                            caretIcon={MenuDownIcon as React.ComponentType<{ className: string; onClick?: () => void }>}
+                            extensionsController={props.extensionsController}
+                            link={ExtensionLink}
+                        />
+                    </ShortcutProvider>
                 </li>
             </ul>
         </div>

src/shared/components/ShortcutProvider.tsx

@@ -0,0 +1,42 @@
+import { Context, ContextProvider, ProviderProps, ShortcutManager } from '@slimsag/react-shortcuts'
+import * as React from 'react'
+
+/**
+ * Describes the variable this file injects into the `global` object. It is
+ * heavily prefixed to avoid collisions.
+ */
+interface GlobalContext {
+    /** The singleton ShortcutManager object. */
+    browserExtensionShortcutManager?: ShortcutManager
+}
+
+// This ShortcutProvider is derived from the default @shopify/react-shortcuts
+// implementation:
+//
+// https://github.com/Shopify/quilt/blob/master/packages/react-shortcuts/src/ShortcutProvider/ShortcutProvider.tsx
+//
+// We cannot use the default implementation above because it assumes the
+// application is rendered via a single React component in order to create the
+// ShortcutManager singleton. In our case, there are multiple React components
+// on the page and they each need to use a single ShortcutManager, so we must
+// manage it ourselves here. If we did not do this, we would have multiple
+// ShortcutManagers and each would register their own conflicting document
+// event handlers.
+export class ShortcutProvider extends React.Component<ProviderProps, never> {
+    public componentDidMount(): void {
+        const globals = global as GlobalContext
+        if (!globals.browserExtensionShortcutManager) {
+            globals.browserExtensionShortcutManager = new ShortcutManager()
+            globals.browserExtensionShortcutManager.setup()
+        }
+    }
+
+    public render(): JSX.Element | null {
+        const globals = global as GlobalContext
+        const context: Context = {
+            shortcutManager: globals.browserExtensionShortcutManager,
+        }
+
+        return <ContextProvider value={context}>{this.props.children}</ContextProvider>
+    }
+}

yarn.lock

@@ -1306,6 +1306,13 @@
   dependencies:
     prop-types "^15.6.2"
 
+"@slimsag/react-shortcuts@^1.2.1":
+  version "1.2.1"
+  resolved "https://registry.yarnpkg.com/@slimsag/react-shortcuts/-/react-shortcuts-1.2.1.tgz#a3ea054a057137de8636bc9fabac61369103e597"
+  integrity sha512-dUTQjBX1yjnHbqkTL5ditqtx5R6QhcXI3lCnjOy8e5XUOr5LOtVpol1mufU2lAb7DtEYrSVRQpUeNmZrTEgLcg==
+  dependencies:
+    prop-types "^15.6.2"
+
 "@sourcegraph/codeintellify@^3.9.0":
   version "3.9.0"
   resolved "https://registry.yarnpkg.com/@sourcegraph/codeintellify/-/codeintellify-3.9.0.tgz#1e3f002558cc367e8c5ee9b0a54e0c47c6ee8761"