feat: introduce children function to shorthands (#4029)

* feat: introduce `children` function to shorthands

* update docs

* more updates

* Update src/generic.d.ts

Author: layershifter

docs/src/pages/ShorthandProps.mdx

@@ -19,8 +19,19 @@ There are several forms of shorthand values that can be provided, but all of the
 Each component's shorthand has an associated default element type. For example, by default, there is `<Icon />` element rendered for `Button`'s icon shorthand. It is possible to customize props of this default element by providing props object as shorthand value:
 
 ```jsx
-// 💡 'color' and 'name' will be used as <Icon /> element's props
-<Button content='Like' icon={{ color: 'red', name: 'like' }} />
+<>
+  {/* 💡 'color' and 'name' will be used as <Icon /> element's props */}
+  <Button content='Like' icon={{ color: 'red', name: 'like' }} />
+  {/* 💡 you can also add handlers and any DOM props to shorthands */}
+  <Input
+    action={{
+      icon: 'search',
+      onClick: () => console.log('An action was clicked!'),
+    }}
+    actionPosition='left'
+    placeholder='Search...'
+  />
+</>
 ```
 
 ## String as value
@@ -52,7 +63,11 @@ This works because `name` is the default prop of shorthand's `<Icon />` element.
 It is also possible to pass falsy values (`false`, `null` or `undefined`) - in that case there will be nothing rendered for the component's shorthand.
 
 ```jsx
-<Dropdown icon={null} />
+<>
+  {/* 💡 hides a toogle icon in `Dropdown` */}
+  <Dropdown icon={null} />
+  <Dropdown icon={false} />
+</>
 ```
 
 ## React Element as value
@@ -68,9 +83,9 @@ There are cases where it might be necessary to customize the element tree that w
   <Message.Content>
     There is a very important caveat here, though: whenever React Element is directly used as a
     shorthand value, all props that Semantic UI React has created for the shorthand's Component will
-    be spread on the passed element. This means that the provided element should be able to handle props
-    - while this requirement is satisfied for all SUIR components, you should be aware of that when
-    either HTML or any third-party elements are provided.
+    be spread on the passed element. This means that the provided element should be able to handle
+    props - while this requirement is satisfied for all SUIR components, you should be aware of that
+    when either HTML or any third-party elements are provided.
   </Message.Content>
 </Message>
 
@@ -82,26 +97,47 @@ Due to this limitation, you should strive to use other options for shorthand val
 
 However, there still might be cases where it would be impossible to use the object form of the shorthand value - for example, you might want to render some custom elements tree for the shorthand. In that case, function value should be used.
 
-## Function as value
+## Render props via `children`
 
-Providing function as a shorthand value is the most involving but, at the same time, the most powerful option for customizing component's shorthand. The only requirements for this function are:
+Providing function as a shorthand value is the most involving but, at the same time, the most powerful option for customizing component's shorthand. It should return React Element as a result or `null`.
 
-- it should finish synchronously
-- it should return React Element as a result
+```jsx
+<Button
+  content='Like'
+  icon={{
+    children: (Component, componentProps) => <Component {...componentProps} color='red' />,
+    name: 'question',
+  }}
+/>
+```
 
-Thus, in its simplest form, it could be used the following way:
+### Customizing rendered shorthand
+
+There is another use case when render function is very useful for - this is the case where custom element's tree should be rendered for the shorthand. As you might recall, there is a problem that might happen when React Element is provided directly as shorthand value - in that case, props are not propagated to rendered. In order to avoid that the following strategy should be considered:
 
 ```jsx
 <Button
   content='Like'
-  icon={(Component, componentProps) => <Component {...componentProps} color='red' name='like' />}
+  icon={{ children: (Component, componentProps) => <Label basic>+1</Label> }}
 />
 ```
 
-## Customizing rendered shorthand
+## Function as value (_deprecated_)
 
-There is another use case when render function is very useful for - this is the case where custom element's tree should be rendered for the shorthand. As you might recall, there is a problem that might happen when React Element is provided directly as shorthand value - in that case, props are not propagated to rendered. In order to avoid that the following strategy should be considered:
+<Message warning>
+  This usage is deprecated and will be removed in v3, please use render props instead.
+</Message>
+
+Providing function as a shorthand value is the most involving but, at the same time, the most powerful option for customizing component's shorthand. The only requirements for this function are:
+
+- it should finish synchronously
+- it should return React Element as a result
+
+Thus, in its simplest form, it could be used the following way:
 
 ```jsx
-<Button content='Like' icon={(Component, componentProps) => <Label basic>+1</Label>} />
+<Button
+  content='Like'
+  icon={(Component, componentProps) => <Component {...componentProps} color='red' name='like' />}
+/>
 ```

package.json

@@ -39,7 +39,7 @@
     "test": "cross-env NODE_ENV=test node -r @babel/register ./node_modules/karma/bin/karma start karma.conf.babel.js",
     "test:watch": "yarn test --no-single-run",
     "test:umd": "gulp build:dist:umd && node test/umd.js",
-    "tsd:test": "gulp build:dist:commonjs:tsd && tsc -p ./ && rimraf test/typings.js"
+    "tsd:test": "gulp build:dist:commonjs:tsd && tsc -p ./ --noEmit"
   },
   "husky": {
     "hooks": {

src/generic.d.ts

@@ -56,18 +56,31 @@ export interface StrictHtmlSpanProps {
 // Types
 // ======================================================
 
+/**
+ * @deprecated Will be removed in v3
+ */
 export type SemanticShorthandItemFunc<TProps> = (
-  component: React.ReactType<TProps>,
+  component: React.ElementType<TProps>,
   props: TProps,
   children?: React.ReactNode | React.ReactNodeArray,
 ) => React.ReactElement<any> | null
 
+export type ShorthandRenderFunction<C extends React.ElementType, P> = (
+  Component: C,
+  props: P,
+) => React.ReactNode
+
 export type SemanticShorthandCollection<TProps> = SemanticShorthandItem<TProps>[]
 export type SemanticShorthandContent = React.ReactNode
-export type SemanticShorthandItem<TProps> =
+export type SemanticShorthandItem<TProps extends Record<string, any>> =
   | React.ReactNode
-  | TProps
   | SemanticShorthandItemFunc<TProps>
+  | (Omit<TProps, 'children'> & {
+      // Not all TProps can have `children`, without this condition it will match to "any"
+      children?: TProps extends { children: any }
+        ? TProps['children'] | ShorthandRenderFunction<React.ElementType<TProps>, TProps>
+        : ShorthandRenderFunction<React.ElementType<TProps>, TProps>
+    })
 
 // ======================================================
 // Styling

src/lib/factories.js

@@ -1,7 +1,9 @@
-import _ from 'lodash'
 import cx from 'clsx'
+import _ from 'lodash'
 import * as React from 'react'
 
+const DEPRECATED_CALLS = {}
+
 // ============================================================
 // Factories
 // ============================================================
@@ -22,8 +24,11 @@ export function createShorthand(Component, mapValueToProps, val, options = {}) {
   if (typeof Component !== 'function' && typeof Component !== 'string') {
     throw new Error('createShorthand() Component must be a string or function.')
   }
+
   // short circuit noop values
-  if (_.isNil(val) || _.isBoolean(val)) return null
+  if (_.isNil(val) || _.isBoolean(val)) {
+    return null
+  }
 
   const valIsString = _.isString(val)
   const valIsNumber = _.isNumber(val)
@@ -108,15 +113,35 @@ export function createShorthand(Component, mapValueToProps, val, options = {}) {
   // ----------------------------------------
 
   // Clone ReactElements
-  if (valIsReactElement) return React.cloneElement(val, props)
+  if (valIsReactElement) {
+    return React.cloneElement(val, props)
+  }
+
+  if (typeof props.children === 'function') {
+    return props.children(Component, { ...props, children: undefined })
+  }
 
   // Create ReactElements from built up props
   if (valIsPrimitiveValue || valIsPropsObject) {
     return React.createElement(Component, props)
   }
 
   // Call functions with args similar to createElement()
-  if (valIsFunction) return val(Component, props, props.children)
+  // TODO: V3 remove the implementation
+  if (valIsFunction) {
+    if (process.env.NODE_ENV !== 'production') {
+      if (!DEPRECATED_CALLS[Component]) {
+        DEPRECATED_CALLS[Component] = true
+
+        // eslint-disable-next-line no-console
+        console.warn(
+          `Warning: There is a deprecated shorthand function usage for "${Component}". It is deprecated and will be removed in v3 release. Please follow our upgrade guide: https://github.com/Semantic-Org/Semantic-UI-React/pull/4029`,
+        )
+      }
+    }
+
+    return val(Component, props, props.children)
+  }
   /* eslint-enable react/prop-types */
 }
 

test/specs/lib/factories-test.js

@@ -433,44 +433,80 @@ describe('factories', () => {
       itOverridesDefaultPropsWithFalseyProps('props object', {
         value: { undef: undefined, nil: null, zero: 0, empty: '' },
       })
+
+      describe('children', () => {
+        it('is called once', () => {
+          const children = sandbox.spy()
+
+          getShorthand({ value: { children } })
+          children.should.have.been.calledOnce()
+        })
+
+        it('is called with Component, props, children', () => {
+          const children = sandbox.spy(() => <div />)
+
+          getShorthand({ Component: 'p', value: { children } })
+          children.should.have.been.calledWithExactly('p', { children: undefined })
+        })
+
+        it('receives defaultProps in its props argument', () => {
+          const children = sandbox.spy(() => <div />)
+          const defaultProps = { defaults: true }
+
+          getShorthand({ Component: 'p', defaultProps, value: { children } })
+          children.should.have.been.calledWithExactly('p', { ...defaultProps, children: undefined })
+        })
+
+        it('receives overrideProps in its props argument', () => {
+          const children = sandbox.spy(() => <div />)
+          const overrideProps = { overrides: true }
+
+          getShorthand({ Component: 'p', overrideProps, value: { children } })
+          children.should.have.been.calledWithExactly('p', {
+            ...overrideProps,
+            children: undefined,
+          })
+        })
+      })
     })
 
+    // TODO: V3 remove this test
     describe('from a function', () => {
+      beforeEach(() => {
+        consoleUtil.disableOnce()
+      })
+
       itReturnsAValidElement(() => <div />)
       itDoesNotIncludePropsFromMapValueToProps(() => <div />)
 
       it('is called once', () => {
         const spy = sandbox.spy()
 
         getShorthand({ value: spy })
-
         spy.should.have.been.calledOnce()
       })
 
       it('is called with Component, props, children', () => {
-        const spy = sandbox.spy(() => <div />)
-
-        getShorthand({ Component: 'p', value: spy })
+        const value = sandbox.spy(() => <div />)
 
-        spy.should.have.been.calledWithExactly('p', {}, undefined)
+        getShorthand({ Component: 'p', value })
+        value.should.have.been.calledWithExactly('p', {}, undefined)
       })
 
       it('receives defaultProps in its props argument', () => {
-        const spy = sandbox.spy(() => <div />)
+        const value = sandbox.spy(() => <div />)
         const defaultProps = { defaults: true }
 
-        getShorthand({ Component: 'p', defaultProps, value: spy })
-
-        spy.should.have.been.calledWithExactly('p', defaultProps, undefined)
+        getShorthand({ Component: 'p', defaultProps, value })
+        value.should.have.been.calledWithExactly('p', defaultProps, undefined)
       })
 
       it('receives overrideProps in its props argument', () => {
-        const spy = sandbox.spy(() => <div />)
+        const value = sandbox.spy(() => <div />)
         const overrideProps = { overrides: true }
 
-        getShorthand({ Component: 'p', overrideProps, value: spy })
-
-        spy.should.have.been.calledWithExactly('p', overrideProps, undefined)
+        getShorthand({ Component: 'p', overrideProps, value })
+        value.should.have.been.calledWithExactly('p', overrideProps, undefined)
       })
     })
 

test/specs/modules/Search/Search-test.js

@@ -8,7 +8,7 @@ import SearchCategory from 'src/modules/Search/SearchCategory'
 import SearchResult from 'src/modules/Search/SearchResult'
 import SearchResults from 'src/modules/Search/SearchResults'
 import * as common from 'test/specs/commonTests'
-import { domEvent, sandbox } from 'test/utils'
+import { consoleUtil, domEvent, sandbox } from 'test/utils'
 
 let attachTo
 let options
@@ -743,6 +743,9 @@ describe('Search', () => {
     })
 
     it(`will not merge for a function`, () => {
+      // TODO: V3 remove this test and simplify the implementation
+      consoleUtil.disableOnce()
+
       wrapperMount(<Search input={{ input: (Component, props) => <Component {...props} /> }} />)
       const input = wrapper.find('input')
 

test/typings.tsx

@@ -1,32 +1,49 @@
 import * as React from 'react'
 import { Button, Dropdown } from '../index'
 
-export const BasicAssert = () => <Button />
+export const BasicAssert = () => (
+  <>
+    <Button />
+    <Button content='Foo' />
+  </>
+)
 
 export const ShorthandItemElementAssert = () => (
   <Dropdown additionLabel={<i style={{ color: 'red' }}>Custom Language: </i>} />
 )
 
 export const ShorthandItemFuncAssert = () => (
-  <Button
-    content='Foo'
-    icon={(Component, props) => (
-      <div className='bar'>
-        <Component name={props.name} />
-      </div>
-    )}
-  />
+  <>
+    <Button
+      icon={{
+        children: (Component, props) => (
+          <div className='bar'>
+            <Component name={props.name} />
+          </div>
+        ),
+      }}
+    />
+    <Button
+      label={{
+        children: (Component, props) => (
+          <div className='bar'>
+            <Component active={props.active}>{props.children}</Component>
+          </div>
+        ),
+      }}
+    />
+    <Button label={{ children: <div className='bar' /> }} />
+  </>
 )
 
-export const ShorthandItemFuncChildren = () => (
-  <Button
-    content='Foo'
-    label={(Component, props, children) => (
-      <div className='bar'>
-        <Component active={props.active}>{children}</Component>
-      </div>
-    )}
-  />
+export const ShorthandItemFuncNullAssert = () => (
+  <Button content='Foo' icon={{ children: () => null }} />
 )
 
-export const ShorthandItemFuncNullAssert = () => <Button content='Foo' icon={() => null} />
+export const ShorthandItemBooleanAssert = () => (
+  <>
+    <Button icon />
+    <Button icon={false} />
+    <Button label={false} />
+  </>
+)