Skip to content

Code Insights: Move insight loading logic to the insight component

Administrator requested to merge code-insights/insight-load-logic-refactoring into main

Created by: vovakulikov

Closes https://github.com/sourcegraph/sourcegraph/issues/23187 Closes https://github.com/sourcegraph/sourcegraph/issues/23203

Context

You can find a quick overview document about this problem here.

For the first version of this new loading logic, we still can use our extension API, but for further steps, we can adopt the apollo client for insight fetching logic. Here is a small apollo link example for it analog for the callViewProvidersInParallel API.

The problem with the current implementation of this system is stateless.

const views = useObservable(
  useMemo(() => 
   getInsightCombinedViews(extensionsController?.extHostAPI, allInsightIds, backendInsightIds), [
    allInsightIds,
    backendInsightIds,
    extensionsController,
    getInsightCombinedViews,
  ])
)

The problem with stateless is that if we change something about one insight of some active insights (views list), this edit operation triggers re-fetching all active insights. Also, you can read about this in this doc.

This PR implementation

This PR adds the useParallelRequest hook approach to parallelize gql and extension-based requests to load insight data. You can find the usePrallelRequest implementation in this PR, but also, you can try the small repositories demo here https://github.com/vovakulikov/apollo-parallel. (yarn serve command to start)

In this PR, we load insight data inside of insight components (Backend Insight, Extension Insight).

image

Insight page These "smart" insights are rendered by the new view grid component SmartInsightViewGrid component

Insight page request waterfall

Screenshot 2021-07-28 at 21 46 25

Home page and directory page At the same time, we want to preserve currently stateless load logic for the home (search) and directory pages. And because of this, we have StaticInsightViewGrid component. This component takes loaded insight objects as a prop and renders them in stateless (visual only) insight only component.

In the future, we can improve loading for the search page or directory page but currently, these pages don't have the re-fetching problem since these pages render only read-only insight grid component (no delete, no any insight modification, that means no re-fetch problem)

What have been done

  • Implement call parallel hook based logic and use instead of callViewProvidersInParallel stream
  • Extend extension API by findInsightPageViewProviderById method.
  • Separate BE insight and extension based insight logic and move it to separate insight components
  • Support homepage and directory page views with the new loading logic

You can find unit tests for the useParallelRequests hook here

Merge request reports