Angular to React Migration: Leaf Components

[stacey-gammon]

Migrating leaf components

1. Identify stateless leaf components
2. Create react component.
   • Put the generic parts into the ui framework
   • Create and run jest tests to ensure 100% test coverage.
   • Generic UI helper logic that isn't an actual component goes into ui_framework/services and should also be accompanied by jest tests.
3. Export your component(s) from the ui_framework by adding it to ui_framework/components/index.js
4. Import your generic react components from the ui_framework by adding them to src/ui/public/react_components.js
5. Create the reactDirective for your generic components in src/ui/public/react_components.js as well.
6. src/core_plugins/kibana/public/kibana.js includes ui/react_components, so if your code lies under there, you're all set to use it in html, just like an angular directive. If it's outside of that folder, you'll have to include src/ui/public/react_components.js.
7. Rinse and Repeat.
   • Pro tip: Work up the DOM, favoring depth over breadth. It's easier to work with react in react and this avoid many scattered react roots.

Goals

• Middle to large sized chunks of react code inside angular
• Prefer stateless components
• Avoid tons of react roots scattered throughout the code
  • Every entry point of react embedded in angular creates a root.

Example React Toolbar Search Box: https://github.com/elastic/kibana/pull/10821

Gotchas

• The angular -> react glue (ngreact) creates a new root for every "entry point". These components can't talk to each other through the react framework.
• I don't think ngreact components handle children, at least they didn't when I was using react-component, but I didn't try when using reactDirective. The following will not fill props.children:

  <react-component name="ButtonComponent">
  <a href="link">
  <react-component name="PlusIconComponent"></react-component>
  </a>
  </react-component>

• When using reactDirective, be sure to use MyComponent.propTypes. This tells the directive what attributes to look for and what props to pass.
• Be careful when passing props that are also valid html attribute names. For example, passing a tooltip prop cause both the react component to display a tooltip, and the angular side of the component as well.

Style Guidance

Prefer Stateless functional components where possible.

When it isn't possible, use ES6 style React Classes.

Prefer reactDirective over react-component

Don't include spaces when embedding brackets inside html.

Prefer

function MyButton({ onClick, children }) {
  return <Button onClick={onClick}>{children}</Button>;
}

over

function MyButton({ onClick, children }) {
  return <Button onClick={ onClick }>{ children }</Button>;
}

This is the opposite of our current eslint rule to use spaces (notice both examples use spaces for the destructured props. This is open for debate but I started with the latter and into an issue because react retained the spaces (can't have #text inside a td).

When to prefix ui_framework elements with kui?

export const KuiTable = ({ children }) => <table className="kuiTable">{children}</table>

I'm not sure where I stand on this one. One option is to do this with the simplest elements, but once the elements get more complicated, dropping the kui prefix, but this is inconsistent and it's unclear where to draw the line. My only worry is creating long names for elements. e.g. kui_landing_page_table or landing_page_table.

Action function names and prop function names

Name action functions in the form of a strong verb and passed properties in the form of on\<Subject>\<Change>. E.g:

<sort-button onClick={action.sort}/>
<pagerButton onPageNext={action.turnToNextPage} />

Avoid creating a function and passing that as a property, in render functions.

E.g. avoid:

<LandingPageTable
    onNextPage={() => ItemTableActions.turnToNextPage(itemTableState)}
    onPreviousPage={() => ItemTableActions.turnToPreviousPage(itemTableState)}
  >
  </LandingPageTable>;

Background: https://facebook.github.io/react/docs/handling-events.html

Prefer primitives over objects when storing in state.

E.g. prefer:

this.setState({
  currentPage: 0,
  selectedIds: []
});

instead of:

this.setState({
  pager: new Pager(),
  selectedIds: new SelectedIds()
});

[cjcenizal]

As part of the style guide, we should also decide whether or not to namespace functions by exporting an object, or to directly export the named functions (per @kjbekkelund's suggestion https://github.com/elastic/kibana/pull/10259#discussion_r100425175).

[cjcenizal]

Here are some other ideas explored in https://github.com/Stacey-Gammon/kibana/pull/3:

• Emphasizing primitives makes code easier to follow. If our data consists of primitive values, then it's easier to create pure functions for operating on them. They're explicit, so the code communicates its meaning without requiring you to learn an abstraction. This also lends itself well to the idea of only assigning primitives to the state object, and not class instances.
• Pure functions are easier to understand. I don't want to have to think about side effects or mutated state. When invoking a pure function, all I have to think about is what goes in and what comes out.
• I think passing props multiple times down the component tree is a code smell. I'd like to hear what @kjbekkelund thinks about this and what kinds of solutions they used on the Cloud team. Personally, I think that this is a sign that we should be using composition more and passing props less.

And as noted in https://github.com/elastic/kibana/pull/10434#pullrequestreview-23334961, I think we should follow the process of gradual abstraction. Let's write React views with all of the necessary logic for handling user input and deriving state encapsulated within the view, without creating new abstractions. This also means writing large render methods, with the UI composed out of fine-grained components (which will live within the UI Framework). Then, in subsequent iterations, look for abstractions, ways to de-duplicate logic, and generalize patterns in the UI.

[cjcenizal]

Closing in favor of https://github.com/elastic/eui/issues/500.