FormInput: Support for defaultProps will be removed [v3]

[DroopeGu1]

Hello, I am using the library in version "semantic-ui-css": "^2.5.0", and "semantic-ui-react": "^2.1.4", along with react 18 and next 13 , and I am having the following warning or error, I searched but found no solution, would not be compatible with the version I use?

[welcome]

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you've completed all the fields in the issue template so we can best help.

We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

[layershifter]

FYI It's not only about FormInput, there is a lot of components that use defaultProps, for example:

Semantic-UI-React/src/modules/Search/Search.js

Lines 682 to 688 in 7d3e162

	Search.defaultProps = {
	icon: 'search',
	input: 'text',
	minCharacters: 1,
	noResultsMessage: 'No results found.',
	showNoResults: true,
	}

Semantic-UI-React/src/addons/Radio/Radio.js

Lines 36 to 38 in 7d3e162

	Radio.defaultProps = {
	type: 'radio',
	}

Semantic-UI-React/src/views/Feed/FeedUser.js

Lines 38 to 40 in 7d3e162

	FeedUser.defaultProps = {
	as: 'a',
	}

Semantic-UI-React/src/views/Feed/FeedLike.js

Lines 34 to 36 in 7d3e162

	FeedLike.defaultProps = {
	as: 'a',
	}

Semantic-UI-React/src/elements/Icon/IconGroup.js

Lines 43 to 45 in 7d3e162

	IconGroup.defaultProps = {
	as: 'i',
	}

Actions

• Update all components not use defaultProps, for example

  const Radio = React.forwardRef(function (props, ref) {
  - const { slider, toggle, type } = props
  + const { slider, toggle, type = 'radio' } = props
  
    const rest = getUnhandledProps(Radio, props)
  
  // ---
  
  -Radio.defaultProps = {
  -  type: 'radio',
  -}

• Add new conformance test to ensure that .defaultProps are never used

  // test/specs/commonTests/isConformant.js
  
  +  // ----------------------------------------
  +  // Has no deprecated .defaultProps
  +  // ----------------------------------------
  +  describe('_meta', () => {
  +    it('does not exist', () => {
  +      expect(Component.defaultProps).to.be.undefined()
  +    })
  +  })

[rsodre]

Anyone know of a way to suppress that specific warning or some react version to rollback?

Or anyone working on a PR?
If not, I'm almost taking it.

This is soooo annoying!
My work feels like that episode in Seinfeld where Kramer had a big red neon sign by his window.

[layershifter]

Or anyone working on a PR?
If not, I'm almost taking it.

Nope, this issue waits for the hero 🦸

[rsodre]

I'll try to find some time to do it.
Will reply here again if I start.

Meanwhile...
https://stackoverflow.com/a/64197014/360930

[achimkoellner]

We experience the warning with these components

Table
Table.Header
Table.Body
Table.Footer
Table.Row
Table.Cell
Table.HeaderCell

[joequery]

@layershifter I've attempted to start work on this. On my very first attempt at updating MessageItem, removing the defaultProps and changing the function to look like

  const MessageItem = React.forwardRef(function (props, ref) {
    const { children, className, content, as="li" } = props

did not seem to actually set the as value. The unittests failed with

FAILED TESTS:
  MessageItem
    ✖ renders an li tag
      Chrome Headless 97.0.4691.0 (Linux x86_64)
      AssertionError: expected <div /> to have a 'li' tag name, but it has 'div'
           HTML:
           <div className="content" />
          at Context.<anonymous> (test/tests.bundle.js:540928:97)

    forwardsRef
      ✖ forwards ref to "li"
        Chrome Headless 97.0.4691.0 (Linux x86_64)
        AssertionError: expected spy to have been called with arguments matching { tagName: "LI" }
        <div class="content"></div> { tagName: "LI" }
            at Context.<anonymous> (test/tests.bundle.js:541688:28)

Any advice here would be appreciated.

[joequery]

So, this does work. Is this method acceptable?

~ const MessageItem = React.forwardRef(function (passedProps, ref) {
+   const defaultProps = { as: 'li' }
+   const props = { ...defaultProps, ...passedProps }
    const { children, className, content } = props

[joequery]

Noting here that removal of defaultProps is going to potentially affect this portion of the shorthand props common test grouping:

    if (alwaysPresent || (Component.defaultProps && Component.defaultProps[propKey])) {
      it(`has default ${name} when not defined`, () => {
        wrapper = mount(React.createElement(Component, requiredProps))

        wrapper.should.have.descendants(ShorthandComponent)
      })
    } 

(Link to code in test/specs/commonTests/implementsShorthandProp.js)

Since these testcases are dynamically generated, it's going to be important that, if reasonable, the number of tests asserted is the same before and after these changes. Failing to generate a testcase would result in a silent failure.

[layershifter]

@layershifter I've attempted to start work on this. On my very first attempt at updating MessageItem, removing the defaultProps and changing the function to look like

@joequery thanks for looking to it and sorry for delay in the reply.

My proposal is to replace getElementType with getComponentType that things better:

// src/collections/Message/MessageItem.js
-  const ElementType = getElementType(MessageItem, props)
-  const ElementType = getComponentType(props, { defaultAs: 'li' })

// src/lib/index.js
+export getComponentType from './getComponentType'
export getElementType from './getElementType'

// src/lib/getComponentType.js
/**
 * Returns a createElement() type based on the props of the Component.
 * Useful for calculating what type a component should render as.
 *
 * @param {object} props A ReactElement props object
 * @param {object} [options={}]
 * @param {function} [options.defaultAs] A default element type.
 * @param {function} [options.getDefault] A function that returns a default element type.
 * @returns {string|function} A ReactElement type
 */
function getComponentType(props, options = {}) {
  const { defaultAs, getDefault } = options

  // ----------------------------------------
  // user defined "as" element type

  if (props.as && props.as !== defaultAs) return props.as

  // ----------------------------------------
  // computed default element type

  if (getDefault) {
    const computedDefault = getDefault()
    if (computedDefault) return computedDefault
  }

  // ----------------------------------------
  // infer anchor links

  if (props.href) return 'a'

  // ----------------------------------------
  // use defaultProp or 'div'

  return defaultAs || 'div'
}

export default getComponentType

in the end, getElementType will be removed. Does it make sense?