Redesign plug API (#16)

Author: ovidiuch

TODO.md

@@ -1,3 +1,11 @@
+- [x] Rename Slot props to `slotProps`
+- [x] Test new plug API in react-cosmos with yarn link
+- [x] Test for context memoization in plugs
+- [ ] Create useArraySlot
+- [ ] Maybe: createPluginStore
+
+---
+
 - [x] Use TypeScript
 - [x] Use ui-plugin
   - [x] Map plugs per plugin
@@ -6,5 +14,5 @@
 - [x] Only render plugs of enabled plugins
 - [x] Support children in plugs
 - [x] Publish Flow types
-- [ ] Export types for PluginsConsumer
-- [ ] Handle "dispatch" props in getProps (avoid re-rendering because anonymous function gets created on every getProps call)
+- [ ] ~~Export types for PluginsConsumer~~
+- [ ] ~~Handle "dispatch" props in getProps (avoid re-rendering because anonymous function gets created on every getProps call)~~

package.json

@@ -20,23 +20,21 @@
     "@skidding/linked-list": "^1.0.2",
     "array-find-index": "^1.0.2",
     "lodash": "^4.17.11",
-    "react-is": "^16.6.3",
     "ui-plugin": "^2.0.0-alpha.2"
   },
   "devDependencies": {
-    "@skidding/async-retry": "^1.0.2",
-    "@types/jest": "^23.3.10",
-    "@types/lodash": "^4.14.119",
-    "@types/react": "^16.7.17",
-    "@types/react-is": "^16.5.0",
-    "@types/react-test-renderer": "^16.0.3",
-    "jest": "^23.6.0",
-    "prettier": "^1.15.3",
-    "react": "^16.6.3",
-    "react-test-renderer": "^16.6.3",
-    "ts-jest": "^23.10.5",
-    "tslint": "^5.11.0",
-    "tslint-config-prettier": "^1.17.0",
-    "typescript": "^3.2.2"
+    "@skidding/async-retry": "^2.0.0",
+    "@types/jest": "^24.0.12",
+    "@types/lodash": "^4.14.123",
+    "@types/react": "^16.8.17",
+    "@types/react-test-renderer": "^16.8.1",
+    "jest": "^24.8.0",
+    "prettier": "^1.17.0",
+    "react": "^16.8.6",
+    "react-test-renderer": "^16.8.6",
+    "ts-jest": "^24.0.2",
+    "tslint": "^5.16.0",
+    "tslint-config-prettier": "^1.18.0",
+    "typescript": "^3.4.5"
   }
 }

src/Slot/PlugConnect.tsx

@@ -1,40 +1,20 @@
-import { isEqual } from 'lodash';
 import * as React from 'react';
 import { getPlugin, getPluginContext, onStateChange } from 'ui-plugin';
-import { GetProps } from '../types';
+import { PlugComponentType } from '../types';
 
 type Props = {
   pluginName: string;
-  component: React.ComponentType;
+  component: PlugComponentType<any, any>;
   slotProps: object;
-  getProps: GetProps;
   children?: React.ReactNode;
 };
 
-type State = {
-  plugProps: object;
-};
-
-export class PlugConnect extends React.Component<Props, State> {
-  static getDerivedStateFromProps(props: Props, state: State) {
-    const plugProps = getPlugProps(props);
-
-    if (plugPropsEqual(plugProps, state.plugProps)) {
-      return null;
-    }
-
-    return { plugProps };
-  }
-
-  state = {
-    plugProps: this.getPlugProps(),
-  };
-
+export class PlugConnect extends React.Component<Props> {
   removeStateChangeHandler: null | (() => unknown) = null;
 
   render() {
     const { component, children } = this.props;
-    const { plugProps } = this.state;
+    const plugProps = this.getPlugProps();
 
     return React.createElement(component, plugProps, children);
   }
@@ -57,31 +37,13 @@ export class PlugConnect extends React.Component<Props, State> {
     // rendering is async, it takes a while for the Slot components to process
     // plugin changes, so PlugConnect components might receive state changes
     // for plugins that are no longer enabled.
-    if (!getPlugin(this.props.pluginName).enabled) {
-      return;
-    }
-
-    const newProps = this.getPlugProps();
-
-    // NOTE: This can be optimized. We can avoid running plug.getProps when
-    // relevant state hasn't changed.
-    // TODO: How to avoid comparing annonymous dispatch-like functions that get
-    // created on every getProps call?
-    if (!plugPropsEqual(newProps, this.state.plugProps)) {
-      this.setState({ plugProps: newProps });
+    if (getPlugin(this.props.pluginName).enabled) {
+      this.forceUpdate();
     }
   };
 
   getPlugProps() {
-    return getPlugProps(this.props);
+    const { pluginName, slotProps = {} } = this.props;
+    return { pluginContext: getPluginContext(pluginName), slotProps };
   }
 }
-
-function getPlugProps({ pluginName, slotProps, getProps }: Props) {
-  return getProps(getPluginContext(pluginName), slotProps);
-}
-
-function plugPropsEqual(plugProps1: object, plugProps2: object) {
-  // IDEA: Assume functions with same name are equal
-  return isEqual(plugProps1, plugProps2);
-}

src/Slot/index.tsx

@@ -1,7 +1,6 @@
 import createLinkedList from '@skidding/linked-list';
 import { isEqual } from 'lodash';
 import * as React from 'react';
-import { isValidElementType } from 'react-is';
 import { onPluginLoad } from 'ui-plugin';
 import { Plug } from '../types';
 import { getEnabledPlugsForSlot } from '../store';
@@ -11,7 +10,7 @@ import { PlugConnect } from './PlugConnect';
 type Props = {
   name: string;
   children?: React.ReactNode;
-  props?: object;
+  slotProps?: object;
 };
 
 type State = {
@@ -26,7 +25,7 @@ export class Slot extends React.Component<Props, State> {
   removePluginLoadHandler: null | (() => unknown) = null;
 
   render() {
-    const { name, children, props = {} } = this.props;
+    const { name, children, slotProps = {} } = this.props;
     const { plugs } = this.state;
     const { Provider, Consumer } = getSlotContext(name);
 
@@ -52,7 +51,7 @@ export class Slot extends React.Component<Props, State> {
           }
 
           return (
-            <Provider value={next()}>{getPlugNode(plug, props, children)}</Provider>
+            <Provider value={next()}>{getPlugNode(plug, slotProps, children)}</Provider>
           );
         }}
       </Consumer>
@@ -72,34 +71,19 @@ export class Slot extends React.Component<Props, State> {
 
   handlePluginLoad = () => {
     const newPlugs = getEnabledPlugsForSlot(this.props.name);
-
     if (!isEqual(newPlugs, this.state.plugs)) {
       this.setState({ plugs: newPlugs });
     }
   };
 }
 
 function getPlugNode(plug: Plug, slotProps: object, children?: React.ReactNode) {
-  const { pluginName, render, getProps } = plug;
-
-  if (typeof render === 'string' || !isValidElementType(render)) {
-    return render;
-  }
-
-  if (typeof getProps === 'function') {
-    return (
-      <PlugConnect
-        pluginName={pluginName}
-        component={render}
-        slotProps={slotProps}
-        getProps={getProps}
-      >
-        {children}
-      </PlugConnect>
-    );
-  }
-
-  return React.createElement(render, slotProps, children);
+  const { pluginName, component } = plug;
+  return (
+    <PlugConnect pluginName={pluginName} component={component} slotProps={slotProps}>
+      {children}
+    </PlugConnect>
+  );
 }
 
 function getFirstLinkedPlug(plugs: Plug[]) {

src/__tests__/plugCompose.tsx

@@ -9,44 +9,35 @@ it('composes with plugs previously applied on same slot', () => {
   // The first plug opens up the possibility for a future plug to override
   // it or to compose with it. The latter is happening in this case.
   const p1 = createPlugin({ name: 'test1' });
-  p1.plug({
-    slotName: 'root',
-    render: (
-      <Slot name="root">
-        <span>I was here first</span>
-      </Slot>
-    ),
-  });
+  p1.plug('root', () => (
+    <Slot name="root">
+      <span>I was here first</span>
+    </Slot>
+  ));
   p1.register();
 
   // The second and third plugs continue to allow next plugs to override or
   // compose them, as well as continue to render previous plugs via children
   const p2 = createPlugin({ name: 'test2' });
-  p2.plug({
-    slotName: 'root',
-    render: ({ children }) => (
-      <Slot name="root">
-        <>
-          {children}
-          <span>I was here second</span>
-        </>
-      </Slot>
-    ),
-  });
+  p2.plug('root', ({ children }) => (
+    <Slot name="root">
+      <>
+        {children}
+        <span>I was here second</span>
+      </>
+    </Slot>
+  ));
   p2.register();
 
   const p3 = createPlugin({ name: 'test3' });
-  p3.plug({
-    slotName: 'root',
-    render: ({ children }) => (
-      <Slot name="root">
-        <>
-          {children}
-          <span>I was here third</span>
-        </>
-      </Slot>
-    ),
-  });
+  p3.plug('root', ({ children }) => (
+    <Slot name="root">
+      <>
+        {children}
+        <span>I was here third</span>
+      </>
+    </Slot>
+  ));
   p3.register();
 
   loadPlugins();

src/__tests__/plugDecorate.tsx

@@ -9,22 +9,16 @@ it('decorates any future plugs applied on same slot', () => {
   // The first plug opens up the possibility for a future plugin to override
   // it or to compose with it. The latter is happening in this case.
   const p1 = createPlugin({ name: 'test1' });
-  p1.plug({
-    slotName: 'root',
-    render: (
-      <>
-        <span>I was here first</span>
-        <Slot name="root" />
-      </>
-    ),
-  });
+  p1.plug('root', () => (
+    <>
+      <span>I was here first</span>
+      <Slot name="root" />
+    </>
+  ));
   p1.register();
 
   const p2 = createPlugin({ name: 'test2' });
-  p2.plug({
-    slotName: 'root',
-    render: <span>I was here second</span>,
-  });
+  p2.plug('root', () => <span>I was here second</span>);
   p2.register();
 
   loadPlugins();

src/__tests__/plugOrder.tsx

@@ -43,28 +43,22 @@ it('composes plugs registered outside-in', () => {
 
 function registerPreviewIframe() {
   const { plug, register } = createPlugin({ name: 'preview' });
-  plug({
-    slotName: 'root',
-    render: (
-      <Slot name="root">
-        <Preview />
-      </Slot>
-    ),
-  });
+  plug('root', () => (
+    <Slot name="root">
+      <Preview />
+    </Slot>
+  ));
   register();
 }
 
 function registerNav() {
   const { plug, register } = createPlugin({ name: 'nav' });
-  plug({
-    slotName: 'root',
-    render: ({ children }: { children?: React.ReactNode }) => (
-      <div>
-        <Nav />
-        <Slot name="root">{children}</Slot>
-      </div>
-    ),
-  });
+  plug('root', ({ children }: { children?: React.ReactNode }) => (
+    <div>
+      <Nav />
+      <Slot name="root">{children}</Slot>
+    </div>
+  ));
   register();
 }
 

src/__tests__/plugOverride.tsx

@@ -9,17 +9,11 @@ it('overrides plug previously applied on same slot', () => {
   // The first plugs opens up the possibility for a future plug to override
   // it or to compose with it. The former is happening in this case.
   const p1 = createPlugin({ name: 'test1 ' });
-  p1.plug({
-    slotName: 'root',
-    render: <Slot name="root">I was here first</Slot>,
-  });
+  p1.plug('root', () => <Slot name="root">I was here first</Slot>);
   p1.register();
 
   const p2 = createPlugin({ name: 'test2 ' });
-  p2.plug({
-    slotName: 'root',
-    render: 'I was here second',
-  });
+  p2.plug('root', () => <>I was here second</>);
   p2.register();
 
   loadPlugins();