feat: GitHub uses the new inject method (#186)

* feat: define GitHub code_intelligence functions and objects

* feat: use generic inject code intelligence function

* feat: annotate lazily loaded code views

* refactor: findCodeElements is an operator

* refactor: wip

* feat: listens for new code views

* feat: only on mutation/intersection observer

* refactor: renames

* feat: requestAnimationFrame

* feat: requestAnimationFrame

* feat: clean up subscriptions

* refactor: use rx animation from scheduler

* refactor: Promise.resolve(maybePromise)

* refactor: inline await

* refactor: address review comments

* feat: single code view

* feat: search and comment code snippets

* feat: search and comment code snippets

* feat: put behind feature flag

* chore: tslint

Author: ijsnow

src/browser/types.ts

@@ -12,10 +12,12 @@ export interface PhabricatorMapping {
  */
 export interface FeatureFlags {
     newTooltips: boolean
+    newInject: boolean
 }
 
 export const featureFlagDefaults: FeatureFlags = {
     newTooltips: true,
+    newInject: false,
 }
 
 // TODO(chris) Switch to Partial<StorageItems> to eliminate bugs caused by

src/extension/scripts/inject.tsx

@@ -17,6 +17,7 @@ import {
 import { featureFlags } from '../../shared/util/featureFlags'
 
 import { injectBitbucketServer } from '../../libs/bitbucket/inject'
+import { injectCodeIntelligence } from '../../libs/code_intelligence'
 import { injectGitHubApplication } from '../../libs/github/inject'
 import { injectPhabricatorApplication } from '../../libs/phabricator/app'
 import { injectSourcegraphApp } from '../../libs/sourcegraph/inject'
@@ -37,7 +38,7 @@ function injectApplication(): void {
 
     const href = window.location.href
 
-    const handleGetStorage = (items: StorageItems) => {
+    const handleGetStorage = async (items: StorageItems) => {
         if (items.disableExtension) {
             return
         }
@@ -99,6 +100,14 @@ function injectApplication(): void {
             setSourcegraphUrl(sourcegraphServerUrl)
             injectBitbucketServer()
         }
+
+        if (isGitHub || isPhabricator) {
+            if (await featureFlags.isEnabled('newInject')) {
+                const subscriptions = await injectCodeIntelligence()
+                window.addEventListener('unload', () => subscriptions.unsubscribe())
+            }
+        }
+
         setUseExtensions(items.useExtensions === undefined ? false : items.useExtensions)
     }
 

src/libs/code_intelligence/inject.tsx …/code_intelligence/code_intelligence.tsxsrc/libs/code_intelligence/inject.tsx renamed to src/libs/code_intelligence/code_intelligence.tsx src/libs/code_intelligence/inject.tsx renamed to src/libs/code_intelligence/code_intelligence.tsx

@@ -15,24 +15,78 @@ import { HoverMerged } from '@sourcegraph/codeintellify/lib/types'
 import { toPrettyBlobURL } from '@sourcegraph/codeintellify/lib/url'
 import * as React from 'react'
 import { render } from 'react-dom'
-import { Observable, of, Subject, Subscription } from 'rxjs'
-import { filter, map, withLatestFrom } from 'rxjs/operators'
+import { animationFrameScheduler, Observable, of, Subject, Subscription } from 'rxjs'
+import { filter, map, mergeMap, observeOn, withLatestFrom } from 'rxjs/operators'
 
 import { createJumpURLFetcher } from '../../shared/backend/lsp'
 import { lspViaAPIXlang } from '../../shared/backend/lsp'
 import { ButtonProps, CodeViewToolbar } from '../../shared/components/CodeViewToolbar'
 import { eventLogger, sourcegraphUrl } from '../../shared/util/context'
+import { githubCodeHost } from '../github/code_intelligence'
+import { phabricatorCodeHost } from '../phabricator/code_intelligence'
+import { findCodeViews } from './code_views'
+
+/**
+ * Defines a type of code view a given code host can have. It tells us how to
+ * look for the code view and how to do certain things when we find it.
+ */
+export interface CodeView {
+    /** A selector used by `document.querySelectorAll` to find the code view. */
+    selector: string
+    /** The DOMFunctions for the code view. */
+    dom: DOMFunctions
+    /**
+     * Finds or creates a DOM element where we should inject the
+     * `CodeViewToolbar`. This function is responsible for ensuring duplicate
+     * mounts aren't created.
+     */
+    getToolbarMount?: (codeView: HTMLElement, part?: DiffPart) => HTMLElement
+    /**
+     * Resolves the file info for a given code view. It returns an observable
+     * because some code hosts need to resolve this asynchronously. The
+     * observable should only emit once.
+     */
+    resolveFileInfo: (codeView: HTMLElement) => Observable<FileInfo>
+    /**
+     * In some situations, we need to be able to adjust the position going into
+     * and coming out of codeintellify. For example, Phabricator converts tabs
+     * to spaces in it's DOM.
+     */
+    adjustPosition?: PositionAdjuster
+    /** Props for styling the buttons in the `CodeViewToolbar`. */
+    toolbarButtonProps?: ButtonProps
+}
+
+export type CodeViewWithOutSelector = Pick<CodeView, Exclude<keyof CodeView, 'selector'>>
+
+export interface CodeViewResolver {
+    selector: string
+    resolveCodeView: (elem: HTMLElement) => CodeViewWithOutSelector
+}
 
 /** Information for adding code intelligence to code views on arbitrary code hosts. */
 export interface CodeHost {
     /**
      * The name of the code host. This will be added as a className to the overlay mount.
      */
     name: string
+
     /**
      * The list of types of code views to try to annotate.
      */
-    codeViews: CodeView[]
+    codeViews?: CodeView[]
+
+    /**
+     * Resolve `CodeView`s from the DOM. This is useful when each code view type
+     * doesn't have a distinct selector for
+     */
+    codeViewResolver?: CodeViewResolver
+
+    /**
+     * Checks to see if the current context the code is running in is within
+     * the given code host.
+     */
+    check: () => Promise<boolean> | boolean
 }
 
 export interface FileInfo {
@@ -55,7 +109,6 @@ export interface FileInfo {
      * The revision the code view is at. If a `baseRev` is provided, this value is treated as the head rev.
      */
     rev?: string
-
     /**
      * The repo bath for the BASE side of a diff. This is useful for Phabricator
      * staging areas since they are separate repos.
@@ -78,31 +131,6 @@ export interface FileInfo {
     baseHasFileContents?: boolean
 }
 
-/**
- * Defines a type of code view a given code host can have. It tells us how to
- * look for the code view and how to do certain things when we find it.
- */
-export interface CodeView {
-    /** A selector used by `document.querySelectorAll` to find the code view. */
-    selector: string
-    /** The DOMFunctions for the code view. */
-    dom: DOMFunctions
-    /** Finds or creates a DOM element where we should inject the `CodeViewToolbar`. */
-    getToolbarMount?: (codeView: HTMLElement, part?: DiffPart) => HTMLElement
-    /** Resolves the file info for a given code view. It returns an observable
-     * because some code hosts need to resolve this asynchronously. The
-     * observable should only emit once.
-     */
-    resolveFileInfo: (codeView: HTMLElement) => Observable<FileInfo>
-    /** In some situations, we need to be able to adjust the position going into
-     * and coming out of codeintellify. For example, Phabricator converts tabs
-     * to spaces in it's DOM.
-     */
-    adjustPosition?: PositionAdjuster
-    /** Props for styling the buttons in the `CodeViewToolbar`. */
-    toolbarButtonProps?: ButtonProps
-}
-
 /**
  * Prepares the page for code intelligence. It creates the hoverifier, injects
  * and mounts the hover overlay and then returns the hoverifier.
@@ -196,41 +224,43 @@ function initCodeIntelligence(codeHost: CodeHost): { hoverifier: Hoverifier } {
  * ResolvedCodeView attaches an actual code view DOM element that was found on
  * the page to the CodeView type being passed around by this file.
  */
-export interface ResolvedCodeView extends CodeView {
+export interface ResolvedCodeView extends CodeViewWithOutSelector {
     /** The code view DOM element. */
     codeView: HTMLElement
 }
 
-function findCodeViews(codeViewInfos: CodeView[]): Observable<ResolvedCodeView> {
-    return new Observable<ResolvedCodeView>(observer => {
-        for (const info of codeViewInfos) {
-            const elements = document.querySelectorAll<HTMLElement>(info.selector)
-            for (const codeView of elements) {
-                observer.next({ ...info, codeView })
-            }
-        }
-    })
-}
+function handleCodeHost(codeHost: CodeHost): Subscription {
+    const { hoverifier } = initCodeIntelligence(codeHost)
 
-export function injectCodeIntelligence(codeHostInfo: CodeHost): Subscription {
-    const { hoverifier } = initCodeIntelligence(codeHostInfo)
+    const subscriptions = new Subscription()
 
-    return findCodeViews(codeHostInfo.codeViews).subscribe(
-        ({ codeView, dom, resolveFileInfo, adjustPosition, getToolbarMount, toolbarButtonProps }) =>
-            resolveFileInfo(codeView).subscribe(info => {
+    subscriptions.add(
+        of(document.body)
+            .pipe(
+                findCodeViews(codeHost),
+                mergeMap(({ codeView, resolveFileInfo, ...rest }) =>
+                    resolveFileInfo(codeView).pipe(map(info => ({ info, codeView, ...rest })))
+                ),
+                observeOn(animationFrameScheduler)
+            )
+            .subscribe(({ codeView, info, dom, adjustPosition, getToolbarMount, toolbarButtonProps }) => {
                 const resolveContext: ContextResolver = ({ part }) => ({
                     repoPath: part === 'base' ? info.baseRepoPath || info.repoPath : info.repoPath,
                     commitID: part === 'base' ? info.baseCommitID! : info.commitID,
                     filePath: part === 'base' ? info.baseFilePath! : info.filePath,
                     rev: part === 'base' ? info.baseRev || info.baseCommitID! : info.rev || info.commitID,
                 })
 
-                hoverifier.hoverify({
-                    dom,
-                    positionEvents: of(codeView).pipe(findPositionsFromEvents(dom)),
-                    resolveContext,
-                    adjustPosition,
-                })
+                subscriptions.add(
+                    hoverifier.hoverify({
+                        dom,
+                        positionEvents: of(codeView).pipe(findPositionsFromEvents(dom)),
+                        resolveContext,
+                        adjustPosition,
+                    })
+                )
+
+                codeView.classList.add('sg-mounted')
 
                 if (!getToolbarMount) {
                     return
@@ -253,4 +283,32 @@ export function injectCodeIntelligence(codeHostInfo: CodeHost): Subscription {
                 )
             })
     )
+
+    return subscriptions
+}
+
+async function injectCodeIntelligenceToCodeHosts(codeHosts: CodeHost[]): Promise<Subscription> {
+    const subscriptions = new Subscription()
+
+    for (const codeHost of codeHosts) {
+        const isCodeHost = await Promise.resolve(codeHost.check())
+        if (isCodeHost) {
+            subscriptions.add(handleCodeHost(codeHost))
+        }
+    }
+
+    return subscriptions
+}
+
+/**
+ * Injects all code hosts into the page.
+ *
+ * @returns A promise with a subscription containing all subscriptions for code
+ * intelligence. Unsubscribing will clean up subscriptions for hoverify and any
+ * incomplete setup requests.
+ */
+export async function injectCodeIntelligence(): Promise<Subscription> {
+    const codeHosts: CodeHost[] = [githubCodeHost, phabricatorCodeHost]
+
+    return await injectCodeIntelligenceToCodeHosts(codeHosts)
 }

src/libs/code_intelligence/code_views.ts

@@ -0,0 +1,123 @@
+import { from, merge, Observable, of, Subject } from 'rxjs'
+import { filter, map, mergeMap } from 'rxjs/operators'
+
+import { CodeHost, ResolvedCodeView } from './code_intelligence'
+
+/**
+ * Emits a ResolvedCodeView when it's DOM element is on or about to be on the page.
+ */
+const emitWhenIntersecting = (margin: number) => {
+    const codeViewStash = new Map<HTMLElement, ResolvedCodeView>()
+
+    const intersectingElements = new Subject<HTMLElement>()
+
+    const intersectionObserver = new IntersectionObserver(
+        entries => {
+            for (const entry of entries) {
+                // `entry` is an `IntersectionObserverEntry`,
+                // which has
+                // [isIntersecting](https://developer.mozilla.org/en-US/docs/Web/API/IntersectionObserverEntry/isIntersecting#Browser_compatibility)
+                // as a prop, but TS complains that it does not
+                // exist.
+                if ((entry as any).isIntersecting) {
+                    intersectingElements.next(entry.target as HTMLElement)
+                }
+            }
+        },
+        {
+            rootMargin: `${margin}px`,
+            threshold: 0,
+        }
+    )
+
+    return (codeViews: Observable<ResolvedCodeView>) =>
+        new Observable<ResolvedCodeView>(observer => {
+            codeViews.subscribe(({ codeView, ...rest }) => {
+                intersectionObserver.observe(codeView)
+                codeViewStash.set(codeView, { codeView, ...rest })
+            })
+
+            intersectingElements
+                .pipe(
+                    map(element => codeViewStash.get(element)),
+                    filter(codeView => !!codeView)
+                )
+                .subscribe(observer)
+        })
+}
+
+/**
+ * findCodeViews finds all the code views on a page given a CodeHost. It emits code views
+ * that are lazily loaded as well.
+ */
+export const findCodeViews = (codeHost: CodeHost, watchChildrenModifications = true) => (
+    containers: Observable<Element>
+) => {
+    const codeViewsFromList: Observable<ResolvedCodeView> = containers.pipe(
+        filter(() => !!codeHost.codeViews),
+        mergeMap(container =>
+            from(codeHost.codeViews!).pipe(
+                map(({ selector, ...info }) => ({
+                    info,
+                    matches: container.querySelectorAll<HTMLElement>(selector),
+                }))
+            )
+        ),
+        mergeMap(({ info, matches }) =>
+            of(...matches).pipe(
+                map(codeView => ({
+                    ...info,
+                    codeView,
+                }))
+            )
+        )
+    )
+
+    const codeViewsFromResolver: Observable<ResolvedCodeView> = containers.pipe(
+        filter(() => !!codeHost.codeViewResolver),
+        map(container => ({
+            resolveCodeView: codeHost.codeViewResolver!.resolveCodeView,
+            matches: container.querySelectorAll<HTMLElement>(codeHost.codeViewResolver!.selector),
+        })),
+        mergeMap(({ resolveCodeView, matches }) =>
+            of(...matches).pipe(
+                map(codeView => ({
+                    ...resolveCodeView(codeView),
+                    codeView,
+                }))
+            )
+        )
+    )
+
+    const obs = [codeViewsFromList, codeViewsFromResolver]
+
+    if (watchChildrenModifications) {
+        const possibleLazilyLoadedContainers = new Subject<HTMLElement>()
+
+        const mutationObserver = new MutationObserver(mutations => {
+            for (const mutation of mutations) {
+                if (mutation.type === 'childList' && mutation.target instanceof HTMLElement) {
+                    const { target } = mutation
+
+                    possibleLazilyLoadedContainers.next(target)
+                }
+            }
+        })
+
+        containers.subscribe(container =>
+            mutationObserver.observe(container, {
+                childList: true,
+                subtree: true,
+            })
+        )
+
+        const lazilyLoadedCodeViews = possibleLazilyLoadedContainers.pipe(findCodeViews(codeHost, false))
+
+        obs.push(lazilyLoadedCodeViews)
+    }
+
+    return merge(...obs).pipe(
+        emitWhenIntersecting(250),
+        filter(({ codeView }) => !codeView.classList.contains('sg-mounted'))
+    )
+}

src/libs/code_intelligence/index.ts

@@ -0,0 +1 @@
+export * from './code_intelligence'