Skip to content

Instantly share code, notes, and snippets.

@flash1293

flash1293/readme.md

Last active July 7, 2021 16:47
Show Gist options
  • Save flash1293/ef51b9d97a5522c802ea60e75867eb29 to your computer and use it in GitHub Desktop.
Save flash1293/ef51b9d97a5522c802ea60e75867eb29 to your computer and use it in GitHub Desktop.
Download ZIP
Deangularizing Graph
Raw
readme.md

The Graph app is still using angular for routing and some parts of the state management. These should be removed to get rid of Angular before it becomes unsupported. As this deangularization is about the technical dependency on Angular, the user shouldn't notice any changes - the UI should still render the same HTML elements.

Overview

There are three main parts in Graph which are not migrated yet to the new setup using react, redux and redux-saga:

Originally it was planned to migrate the nodes and edges state into the existing redux state management, but it's not necessary to do this for de-angularizing Graph - it should be postponed for now. The logic is using an OOP approach - an instance of GraphWorkspace is keeping the state of nodes and edges together with methods to manipulate them. Saving and loading this instance is already integrated with the redux based state management using redux-saga.

For the deangularization, the core logic in GraphWorkspace doesn't have to be touched - this part can be migrated later on.

Steps

Based on the complexity and the fact that the UI won't be changed, I don't think an incremental de-angularization like it's done in Discover is worth it - it can be done more easily as a single PR (which might be worked on by multiple people) without functional intermediate states

The following steps have to be taken to remove angular from Graph:

Move inspect panel UI to react

The inspect panel is rendered via angular here: https://github.com/elastic/kibana/blob/61c4e6fd8d7410bfabe3d108211d7aa1d54c7ef5/x-pack/plugins/graph/public/angular/templates/index.html#L6

This part of the template can be turned into a separate component taking the necessary inputs and rendering them using the kibana_react CodeEditor component (like inspector is using it). As it's a separate part of the UI, using EUI components for tabs is probably the easiest approach, but it would be fine to use the same elements from the template with the defined class names as well

Move control panel UI to react

The control panel is rendered via angular here: https://github.com/elastic/kibana/blob/61c4e6fd8d7410bfabe3d108211d7aa1d54c7ef5/x-pack/plugins/graph/public/angular/templates/index.html#L78

This part of the template can be turned into a separate component taking the necessary inputs and rendering them using react. As this is a very dense control component, it should keep using the current HTML structure and css classes (not rewriting using EUI). The only exception to this are the tooltips - these are currently rendered using the bootstrap tooltip library (tooltip attribute). They should be replaced by EuiToolTip components.

Merge main controller into app component

The main controller here https://github.com/elastic/kibana/blob/4584a8b570402aa07832cf3e5b520e5d2cfa7166/x-pack/plugins/graph/public/app.js#L186 should be merged into the existing GraphApp react component, rendering the contents of https://github.com/elastic/kibana/blob/61c4e6fd8d7410bfabe3d108211d7aa1d54c7ef5/x-pack/plugins/graph/public/angular/templates/index.html#L1

The controller code has to be turned into useEffect/useCallback/useState calls for setting up the various callbacks and state.

E.g. the controller is currently initializing the store object - this can be done in a useState initializer call:

const [store] = useState(() => {
    return createGraphStore({
      basePath: getBasePath(),
      addBasePath,
      indexPatternProvider: $scope.indexPatternProvider,
      indexPatterns: $route.current.locals.indexPatterns,
      createWorkspace: (indexPattern, exploreControls) => {
        const options = {
          indexName: indexPattern,
          vertex_fields: [],
          // Here we have the opportunity to look up labels for nodes...
          nodeLabeller: function () {
            //   console.log(newNodes);
          },
          changeHandler: function () {
            //Allows DOM to update with graph layout changes.
            $scope.$apply();
          },
          graphExploreProxy: callNodeProxy,
          // ...
})

Static attributes on the scope turn into simple local constants (like colors): https://github.com/elastic/kibana/blob/4584a8b570402aa07832cf3e5b520e5d2cfa7166/x-pack/plugins/graph/public/app.js#L330

Callbacks get turned into useCallbacks (like nodeClick):

    nodeClick = useCallback(function (n, $event) {
      //Selection logic - shift key+click helps selects multiple nodes
      // Without the shift key we deselect all prior selections (perhaps not
      // a great idea for touch devices with no concept of shift key)
      if (!$event.shiftKey) {
        const prevSelection = n.isSelected;
        // ...
        }, [workspace]);

Dynamic attributes turn into useState (like spymode):

const [spymode, setSpymode] = useState('request');

It has be to made sure the App component re-renders on store changes to pick up any changes - this should be doable using a useObservable hook: https://github.com/streamich/react-use/blob/master/docs/useObservable.md :

const currentStoreState = useObservable(store);

The component should render the general structure of the template linked above:

Re-render react on workspace changes

The GraphWorkspace instance managed by the controller (app component in the future) has a changeHandler callback option which is called if the state within the instance changed and the UI needs to be rerendered: https://github.com/elastic/kibana/blob/4584a8b570402aa07832cf3e5b520e5d2cfa7166/x-pack/plugins/graph/public/app.js#L277

Currently this is calling $scope.$apply to trigger an Angular digest cycle. It should be replaced by triggering a new react render of the main component. I can't think of a clean way to force a re-render every time the changeHandler is called, but one workaround would be to increase a counter:

  function GraphApp() {
    // ...
    [counter, setCounter] = useState(0);
    
    // ...
    createWorkspace({
      changeHandler: () => {
        setCounter(counter++);
      }
    })
  }

This workaround could be removed once the state management is moved from the GraphWorkspace class into the redux state management.

In the redux sagas notifyAngular is called to trigger an angular re-render. It might be still necessary to keep this mechanism (and rename it to notifyReact) because some changes the saga is doing is only encapsulted in the oop GraphWorkspace instance and react wouldn't receive an update. It can use the same setCounter(counter++); mechanism as the workaround described above.

Merge listing page controller into listing component

The listing page is rendered using angular as well - the controller is defined here: https://github.com/elastic/kibana/blob/4584a8b570402aa07832cf3e5b520e5d2cfa7166/x-pack/plugins/graph/public/app.js#L117 It's basically rendering using react already (https://github.com/elastic/kibana/blob/ee0efabbfcb29f8c265992d63fd14a5ab0bc64e0/x-pack/plugins/graph/public/angular/templates/listing_ng_wrapper.html#L1)

What's necessary to be done is to turn the listing controller logic into useEffect/useCallback/useState calls within the already existing Listing component (similar to the main controller): https://github.com/elastic/kibana/blob/921c942befe1ed2793d92c9a68b47cfbf6e35ba4/x-pack/plugins/graph/public/components/listing.tsx#L31

Use react router for routing

Replace the two existing routes for workspace and listing by react-router-dom:

Right now the route definition loads some data as well (the resolve property) https://github.com/elastic/kibana/blob/4584a8b570402aa07832cf3e5b520e5d2cfa7166/x-pack/plugins/graph/public/app.js#L148, this has to be moved into the listing/app component instead into useState/useEffect hooks. It also has a function to set the read only badge correctly, this has to be moved into the component itself in a useEffect hook as well (calling core.chrome.setBadge)

Remove angular stuff

Angular initalization routines in https://github.com/elastic/kibana/blob/a7b0391702c85f7262026a380763845be73d7fa7/x-pack/plugins/graph/public/application.ts#L84 can be removed Destroying the root scope on unmounting of the app gets replaced by unmounting react at the mountpoint. After removing all references, kibana_legacy should be checked whether any files are not used anymore and be removed as well.

Functionality to test

To test, it makes sense to compare behavior before and after the PR - nothing should change. Things to test explitictly:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment