feat: remove hover rendering, only keep hover attachment

Removes the contents from hovers and only uses the hover object to position the tooltip, not to render any content.

Author: sqs

package-lock.json

@@ -31,12 +31,9 @@
   "dependencies": {
     "@sourcegraph/event-positions": "^1.0.0",
     "@sourcegraph/react-loading-spinner": "0.0.7",
-    "highlight.js": "^9.12.0",
     "lodash": "^4.17.10",
-    "marked": "^0.4.0 || ^0.5.0",
     "mdi-react": "^5.1.0",
     "rxjs": "^6.3.3",
-    "sanitize-html": "^1.18.2",
     "ts-key-enum": "^2.0.0",
     "vscode-languageserver-types": "^3.8.2"
   },
@@ -51,15 +48,12 @@
     "@sourcegraph/tslint-config": "^12.0.0",
     "@types/chai": "^4.1.4",
     "@types/fs-extra": "^5.0.3",
-    "@types/highlight.js": "^9.12.3",
     "@types/karma": "^3.0.0",
     "@types/karma-webpack": "^2.0.4",
     "@types/lodash": "^4.14.110",
-    "@types/marked": "^0.4.0",
     "@types/mocha": "^5.2.3",
     "@types/node": "^10.5.0",
     "@types/react": "^16.7.13",
-    "@types/sanitize-html": "^1.14.0",
     "@types/sinon": "^5.0.1",
     "@types/webpack": "^4.4.4",
     "@types/webpack-env": "^1.13.6",

package.json

@@ -1,14 +1,13 @@
 import { LoadingSpinner } from '@sourcegraph/react-loading-spinner'
-import { castArray, upperFirst } from 'lodash'
+import { upperFirst } from 'lodash'
 import AlertCircleOutlineIcon from 'mdi-react/AlertCircleOutlineIcon'
 import CloseIcon from 'mdi-react/CloseIcon'
 import InformationOutlineIcon from 'mdi-react/InformationOutlineIcon'
 import * as React from 'react'
-import { MarkedString, MarkupContent, MarkupKind } from 'vscode-languageserver-types'
-import { asError, ErrorLike, isErrorLike } from './errors'
-import { highlightCodeSafe, renderMarkdown, toNativeEvent } from './helpers'
+import { ErrorLike, isErrorLike } from './errors'
+import { toNativeEvent } from './helpers'
 import { HoveredToken } from './token_position'
-import { HoverMerged, LOADING } from './types'
+import { HoverAttachment, LOADING } from './types'
 
 /** The component used to render a link */
 export type LinkComponent = React.ComponentType<{ to: string } & React.HTMLAttributes<HTMLElement>>
@@ -34,7 +33,7 @@ const ButtonOrLink: React.StatelessComponent<
  */
 export interface HoverOverlayProps<C = {}> {
     /** What to show as contents */
-    hoverOrError?: typeof LOADING | HoverMerged | null | ErrorLike // TODO disallow null and undefined
+    hoverOrError?: typeof LOADING | HoverAttachment | null | ErrorLike // TODO disallow null and undefined
 
     /**
      * The URL to jump to on go to definition.
@@ -82,7 +81,6 @@ const transformMouseEvent = (handler: (event: MouseEvent) => void) => (event: Re
 
 export const HoverOverlay: <C>(props: HoverOverlayProps<C>) => React.ReactElement<any> = ({
     definitionURLOrError,
-    hoveredToken,
     hoverOrError,
     hoverRef,
     linkComponent,
@@ -133,46 +131,8 @@ export const HoverOverlay: <C>(props: HoverOverlayProps<C>) => React.ReactElemen
                         </h4>
                         {upperFirst(hoverOrError.message)}
                     </div>
-                ) : (
-                    // tslint:disable-next-line deprecation We want to handle the deprecated MarkedString
-                    castArray<MarkedString | MarkupContent>(hoverOrError.contents)
-                        .map(value => (typeof value === 'string' ? { kind: MarkupKind.Markdown, value } : value))
-                        .map((content, i) => {
-                            if (MarkupContent.is(content)) {
-                                if (content.kind === MarkupKind.Markdown) {
-                                    try {
-                                        return (
-                                            <div
-                                                className="hover-overlay__content hover-overlay__row e2e-tooltip-content"
-                                                key={i}
-                                                dangerouslySetInnerHTML={{ __html: renderMarkdown(content.value) }}
-                                            />
-                                        )
-                                    } catch (err) {
-                                        return (
-                                            <div className="hover-overlay__row alert alert-danger">
-                                                <strong>
-                                                    <AlertCircleOutlineIcon className="icon-inline" /> Error rendering
-                                                    hover content
-                                                </strong>{' '}
-                                                {upperFirst(asError(err).message)}
-                                            </div>
-                                        )
-                                    }
-                                }
-                                return content.value
-                            }
-                            return (
-                                <code
-                                    className="hover-overlay__content hover-overlay__row e2e-tooltip-content"
-                                    key={i}
-                                    dangerouslySetInnerHTML={{
-                                        __html: highlightCodeSafe(content.value, content.language),
-                                    }}
-                                />
-                            )
-                        })
-                )}
+                ) : null // hover content
+                }
             </div>
         )}
 

src/HoverOverlay.tsx

@@ -1,10 +1,4 @@
-import { highlight, highlightAuto } from 'highlight.js/lib/highlight'
-import marked from 'marked'
 import * as React from 'react'
-import sanitize from 'sanitize-html'
-import { MarkupContent } from 'vscode-languageserver-types'
-import { HoverOverlayProps, isJumpURL } from './HoverOverlay'
-import { HoverMerged } from './types'
 
 /**
  * Returns true if `val` is not `null` or `undefined`
@@ -21,19 +15,6 @@ export const propertyIsDefined = <T extends object, K extends keyof T>(key: K) =
 ): val is K extends any ? ({ [k in Exclude<keyof T, K>]: T[k] } & { [k in K]: NonNullable<T[k]> }) : never =>
     isDefined(val[key])
 
-const isEmptyHover = (hover: HoverMerged | null): boolean =>
-    !hover ||
-    !hover.contents ||
-    (Array.isArray(hover.contents) && hover.contents.length === 0) ||
-    (MarkupContent.is(hover.contents) && !hover.contents.value)
-
-/**
- * Returns true if the HoverOverlay would have anything to show according to the given hover and definition states.
- */
-export const overlayUIHasContent = (state: Pick<HoverOverlayProps, 'hoverOrError' | 'definitionURLOrError'>): boolean =>
-    (!!state.hoverOrError && !(HoverMerged.is(state.hoverOrError) && isEmptyHover(state.hoverOrError))) ||
-    isJumpURL(state.definitionURLOrError)
-
 /**
  * Scrolls an element to the center if it is out of view.
  * Does nothing if the element is in view.
@@ -54,63 +35,6 @@ export const scrollIntoCenterIfNeeded = (container: HTMLElement, content: HTMLEl
     }
 }
 
-/**
- * Escapes HTML by replacing characters like `<` with their HTML escape sequences like `&lt;`
- */
-const escapeHTML = (html: string): string => {
-    const span = document.createElement('span')
-    span.textContent = html
-    return span.innerHTML
-}
-
-/**
- * Attempts to syntax-highlight the given code.
- * If the language is not given, it is auto-detected.
- * If an error occurs, the code is returned as plain text with escaped HTML entities
- *
- * @param code The code to highlight
- * @param language The language of the code, if known
- * @return Safe HTML
- */
-export const highlightCodeSafe = (code: string, language?: string): string => {
-    try {
-        if (language === 'plaintext' || language === 'text') {
-            return escapeHTML(code)
-        }
-        if (language) {
-            return highlight(language, code, true).value
-        }
-        return highlightAuto(code).value
-    } catch (err) {
-        console.warn('Error syntax-highlighting hover markdown code block', err)
-        return escapeHTML(code)
-    }
-}
-
-/**
- * Renders the given markdown to HTML, highlighting code and sanitizing dangerous HTML.
- * Can throw an exception on parse errors.
- */
-export const renderMarkdown = (markdown: string): string => {
-    const rendered = marked(markdown, {
-        gfm: true,
-        breaks: true,
-        sanitize: false,
-        highlight: (code, language) => highlightCodeSafe(code, language),
-    })
-    return sanitize(rendered, {
-        // Allow highligh.js styles, e.g.
-        // <span class="hljs-keyword">
-        // <code class="language-javascript">
-        allowedTags: [...sanitize.defaults.allowedTags, 'span', 'details', 'summary'],
-        allowedAttributes: {
-            span: ['class'],
-            code: ['class'],
-            a: ['href'],
-        },
-    })
-}
-
 /**
  * Converts a synthetic React event to a persisted, native Event object.
  *

src/helpers.test.ts

@@ -18,7 +18,7 @@ import {
 import { HoverOverlayProps } from './HoverOverlay'
 import { findPositionsFromEvents, SupportedMouseEvent } from './positions'
 import { CodeViewProps, DOM } from './testutils/dom'
-import { createHoverMerged, createStubHoverFetcher, createStubJumpURLFetcher } from './testutils/lsp'
+import { createHoverAttachment, createStubHoverFetcher, createStubJumpURLFetcher } from './testutils/lsp'
 import { dispatchMouseEventAtPositionImpure } from './testutils/mouse'
 import { LOADING } from './types'
 
@@ -48,7 +48,6 @@ describe('Hoverifier', () => {
                     fetchHover: createStubHoverFetcher({ range: hoverRange }, LOADER_DELAY + delayTime),
                     fetchJumpURL: () => of(null),
                     pushHistory: noop,
-                    getReferencesURL: () => null,
                 })
 
                 const positionJumps = new Subject<PositionJump>()
@@ -113,7 +112,6 @@ describe('Hoverifier', () => {
                     fetchHover: createStubHoverFetcher(hover, delayTime),
                     fetchJumpURL: createStubJumpURLFetcher(defURL, delayTime),
                     pushHistory: noop,
-                    getReferencesURL: () => null,
                 })
 
                 const positionJumps = new Subject<PositionJump>()
@@ -360,7 +358,6 @@ describe('Hoverifier', () => {
                     fetchHover: createStubHoverFetcher(hover, delayTime),
                     fetchJumpURL: createStubJumpURLFetcher(defURL, delayTime),
                     pushHistory: noop,
-                    getReferencesURL: () => null,
                 })
 
                 const positionJumps = new Subject<PositionJump>()
@@ -406,7 +403,7 @@ describe('Hoverifier', () => {
                     [key: string]: Pick<HoverOverlayProps, 'hoverOrError' | 'definitionURLOrError'>
                 } = {
                     a: { hoverOrError: LOADING, definitionURLOrError: undefined }, // def url is undefined when it is loading
-                    b: { hoverOrError: createHoverMerged(hover), definitionURLOrError: { jumpURL: defURL } },
+                    b: { hoverOrError: createHoverAttachment(hover), definitionURLOrError: { jumpURL: defURL } },
                 }
 
                 // Click https://sourcegraph.sgdev.org/github.com/gorilla/mux@cb4698366aa625048f3b815af6a0dea8aef9280a/-/blob/mux.go#L24:6
@@ -438,7 +435,6 @@ describe('Hoverifier', () => {
                     fetchHover: createStubHoverFetcher(hover),
                     fetchJumpURL: createStubJumpURLFetcher(defURL),
                     pushHistory: noop,
-                    getReferencesURL: () => null,
                 })
 
                 const positionJumps = new Subject<PositionJump>()
@@ -503,7 +499,6 @@ describe('Hoverifier', () => {
                     fetchHover: createStubHoverFetcher(hover),
                     fetchJumpURL: createStubJumpURLFetcher(defURL),
                     pushHistory: noop,
-                    getReferencesURL: () => null,
                 })
 
                 const positionJumps = new Subject<PositionJump>()
@@ -588,7 +583,6 @@ describe('Hoverifier', () => {
                     fetchHover,
                     fetchJumpURL,
                     pushHistory: noop,
-                    getReferencesURL: () => null,
                 })
 
                 const positionJumps = new Subject<PositionJump>()