RFC: Base component

[levithomason]

There is basic functionality that is shared by all components:

• merge props className with SUI classes
• spread unhandled props
• get element type (augmentation)
• define as and className propTypes, most the time also children

Updating components in any of these areas requires updating all components. This was just done to support augmentation, see #414. There are ~1,800 lines added to accomplish this. The grand majority of this work was duplicating the logic and propTypes for all components.

It seems we could use a base component factory to generate a "conformant" base component. It would handle the common functionality for all components. It could make maintenance much easier. It could also give us the ability to better standardize how components are generated.

This would either work similar to the reusable component parts factories in #345, or be a base component that we extend. In either case, the above list of required features would be abstracted into a single place.

[jeffcarbs]

I've been doing some cleanup lately and I noticed this as well. Not sure the best way forward here, but just wanted to bump this thread.

So many of our components, specifically our sub-components, are nearly identical. Here's an example:

-function MessageContent(props) {
+function MessageHeader(props) {
   const { children, className } = props
-  const classes = cx('content', className)
-  const rest = getUnhandledProps(MessageContent, props)
-  const ElementType = getElementType(MessageContent, props)
+  const classes = cx('header', className)
+  const rest = getUnhandledProps(MessageHeader, props)
+  const ElementType = getElementType(MessageHeader, props)

   return <ElementType {...rest} className={classes}>{children}</ElementType>
 }

-MessageContent._meta = {
-  name: 'MessageContent',
+MessageHeader._meta = {
+  name: 'MessageHeader',
   parent: 'Message',
   type: META.TYPES.COLLECTION,
 }

-MessageContent.propTypes = {
+MessageHeader.propTypes = {
   /** An element type to render as (string or function). */
   as: customPropTypes.as,

   /** Primary content. */
-  children: customPropTypes.children(MessageContent),
+  children: customPropTypes.children(MessageHeader),

   /** Additional classes. */
   className: PropTypes.node,
 }

-export default MessageContent
+export default MessageHeader

Literally the only difference (aside from the component name) is the class name applied.

[levithomason]

Yep, for this I've been toying with "reusable component parts", see #345. I made a memoized component part factory, createPartFactory. It creates a class that handles the case you raised, where only the name and className and some _meta info change.

The "base component" I'm thinking of would also be usable in any of our components somehow. Example, maybe it has getClasses and getElementType methods. It could define the most basic propTypes as well, like as and className. The downsides I see are that it means all other components should extend it and therefore they should all be classes. Also, we'd have to rework the docs considerably as they do not parse inheritance or even spreading props.

I'm not really sure if this is a great idea, a base component. Though, the reusable parts for the most basic component parts (sub components) I think surely is.

[hallister]

There's already an idiomatic way of doing this with an Higher Order Components.

export default baseComponent(WrappedComponent, class) {
    return class BaseComponent extends React.Component {
        displayName = WrappedComponent.displayName || 'BaseComponent';

        render() {
           const { className, children, ...props } = this.props;
           const classes = cx(class, className)
           const rest = getUnhandledProps(WrappedComponent, props)
           const ElementType = getElementType(WrappedComponent, props)

           return <WrappedComponent {...rest} className={classes} component={ElementType}>{children}</WrappedComponent>
        }
   }
};

Now your sub-components are vastly simpler:

function MessageHeader(props) {
    const { component: Component, ...rest } = props;
    return <Component {...rest} />;
}

MessageHeader._meta = {
   name: 'MessageHeader',
   parent: 'Message',
   type: META.TYPES.COLLECTION,
}

export baseComponent(MessageHeader, 'header');

[cdaringe]

• @hallister, in your example, should styles perhaps also receive first class treatment? stlyes={defaultsDeep({}, props.styles, componentStyles)}
• i'm still grokking handledProps (xref). i'm wondering why it's a build time process step to generate those vs having a fixed set of prop keys that are supported to be passed down the tree vs a runtime step? a runtime step would be really rad to increase portability, if at all possible

[levithomason]

At least for SUIR right now, there are no styles until #1579 lands.

handledProps is a build time step because propTypes are stripped in production. They are wrapped so that they are removed with dead code elimination. This means, at runtime, you have no interface that tells you which props the component handled.

[sebilasse]

cc. https://github.com/Semantic-Org/Semantic-UI/issues/5420 - while working on a dojo2 integration I also noticed that a shared baseClass for Container and Grid might make sense. Currently I can set the Container properties in Grid but not vice versa.

[levithomason]

Closing for housekeeping.

I have a branch going which will add popup and tooltip props to all components that support can support them. This will introduce the idea of HOCs for library features.