Use forward refs in Components to access ref instances

[mxschmitt]

Feature Request

Problem description

Currently, not all of components properly forward the passed in ref prop to the underlying component. This leads to end-users not being able to access the underlying DOM node and execute their browsers native functions.

For example when you try to execute checkValidity on the form element itself.

Proposed solution

Upgrade the components to forward refs: https://reactjs.org/docs/forwarding-refs.html

MVP

https://codesandbox.io/s/naughty-solomon-fqk4f?fontsize=14

Here you see who examples, the current <Form> component and the native browsers one. The ref instances are totally different.

[patrickp-dev]

Here's a workaround that takes advantage of semantics 'as' prop and prop forwarding to give you access to the rendered DOM element. It'll work with any semantic components, you just need to pass in the render type, which in most cases will probably be the default.

https://codesandbox.io/s/semantic-ui-react-refs-workaround-ibnnv

[jacobweber]

@patrickp-dev Thanks for the workaround, but it seems to be causing some bad side-effects for me. I added an controlled Input and a useState hook to your example:

https://codesandbox.io/s/semantic-ui-react-refs-workaround-2hr0e

But with this workaround, the input loses focus after typing.

Not sure what's happening, but I'm wondering if there's any other way to work around this.

(Use case: I'm trying to capture keyboard events in a Modal, which means I need to focus() the modal's div when it's rendered. So I need a ref to focus. I saw some mention of a Ref component in other tickets, but it seems to be removed.)

[patrickp-dev]

@jacobweber Hey jacob, had a look at your example. The lose of input focus was caused by the fact the 'as' prop was being reassigned a new instance of the higher order component that was returned by the forwardRefAs on each rerender, (my bad). All that's required is pulling the HOC instantiation out of the 'App' component so that it doesn't remount the entire form on each state change. I've updated your example to get it working and have update my original example. https://codesandbox.io/s/semantic-ui-react-refs-workaround-h0f64

[jacobweber]

@patrickp-dev Thanks! I'll give it a try.

[layershifter]

Summary:

• React.forwardRef is only a solution. All components should forward refs properly
• Ref component should removed and deprecated

[layershifter]

https://github.com/Semantic-Org/Semantic-UI-React/issues/3915#issuecomment-665188625

@levithomason should we update the docs to mention that since we have a full major version release that we want to stick to semantic versioning while in maintenance?

Probably we should put a this notice about Semver or a badge in README.md.

@layershifter I think this would also have to be a breaking major version change now that we officially published at 1.0.0 version.

@brianespinosa It's a good question.

I don't think that it will be breaking change for function components as they don't have refs at all. For class components it's it will a breaking change as third party code may rely on some private class methods. What are you suggestions around it?

[layershifter]

@mxschmitt BTW, for the original issue. Ref component can do what you want:

<Ref innerRef={testRef}>
  <Form />
</Ref>

https://codesandbox.io/s/semantic-ui-react-refs-issue-u7zpv?file=/src/index.js

[brianespinosa]

I don't think that it will be breaking change for function components as they don't have refs at all. For class components it's it will a breaking change as third party code may rely on some private class methods. What are you suggestions around it?

I think now that we have released a full version we should stick with semantic versioning as I think that will be everyone's expectation once a full major version is out. I think it is also React team's recommendation for library authors that we should treat this as a breaking change and release a new major version.

[mxschmitt]

@layershifter looks great! For me it would be fine with closing this issue. 👍

[layershifter]

@mxschmitt I will keep this issue opened for tracking work related to React.forwardRef() changes as the huge amount work should be done 🐤

[layershifter]

  "version": "17.0.0-alpha.0",

https://github.com/facebook/react/blob/2704bb5374e52ed548db96df2d975dae42158dfb/packages/react/package.json#L7 This is a good motivation for changes. Going to start there soon.

[layershifter]

FYI: this #4039 PR restores docs for Ref component and clarifies when it should be used.

[layershifter]

Small update there. SUIR 2.0.0 was released yesterday and I started to work on this item. I will stage a v3 branch soon.

• [x] do not use .state() in Enzyme (done, waits for PR)
• [x] convert existing class components to be functional & use React.forwardRef()
• [ ] deprecate Ref component

[sparrowV]

@layershifter so in order to take care of that warning we should use functional components and React.forwardRef() ? BTW does this warning implies any security/performance issues? Is it ok to leave it in production code?

[dvtkrlbs]

Are there any progress on this ?

[YuriFontella]

I didn't understand if this had any solution, but anyway. I'm trying to use the react-hook-form.

Directly with input this works:

const {register} = useForm ()
<input name="name" ref={register} />

Using the component does not work:

const {register} = useForm ()
<Form.Input name="name" ref={register} />

"Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?"

[layershifter]

@YuriFontella

const {register} = useForm ()
<Form.Input name="name" input={{ ref: register }} />

Should work for your case

[layershifter]

@layershifter so in order to take care of that warning we should use functional components and React.forwardRef() ? BTW does this warning implies any security/performance issues? Is it ok to leave it in production code?

Warnings discussed in this PR are coming from SUIR, there is nothing that you can do with it until changes will be done on library side.

[YuriFontella]

Thanks @layershifter.

And how would it look on other form components?

Form.TextArea Form.Checkbox Form.Select

I was using html instead of components, but the select totally loses the style.

[brandonm]

This question is probably better off for StackOverflow but for components that don't have a native element to ref react-hooks-form provides Controller. Here is an example for something I needed:

        <Controller // react-hook-form (validation)
            control={control}
            name={name}
            defaultValue={value}
            rules={fieldValidators}
            render={({ name: renderName, value: renderValue, onChange }) => (
                <Form.Field error={error}>
                    <label htmlFor={name}>{label}</label>
                    <Select
                        className={className}
                        name={renderName}
                        defaultValue={renderValue}
                        placeholder={placeholder}
                        options={options}
                        error={error}
                        icon={icon}
                        onChange={(event, { value: newValue }) => onChange(newValue)}
                    />
                    {error && (
                        <Label prompt className="ui pointing above prompt">
                            {error.message}
                        </Label>
                    )}
                </Form.Field>
            )}
        />

[YuriFontella]

Edit: Using the Controller with Select and Checkbox.

<Controller
  name="type"
  control={control}
  defaultValue=""
  render={props => 
    <Form.Select 
      onChange={(e, { value }) => props.onChange(value)}
      value={props.value}
      options={[
        { text: 'Privada', value: 'privada' },
        { text: 'ONG', value: 'ong' },
        { text: 'Independente', value: 'independente' },
        { text: 'Sociedade', value: 'sociedade', },
        { text: 'Governamental', value: 'governamental' }
      ]}
    />
  }
/>

<Controller
  name="privacy"
  defaultValue=""
  control={control}
  render={props => 
    <Form.Checkbox radio onChange={(e, { value }) => props.onChange(value)} checked={props.value === 'public'} value="public" />
  }
/>

[ZachHaber]

I had to go a bit further on an application I was working on. I was running into errors because I needed to make the focus exposed to react hook form, plus the added benefit of having it scroll to the offending field is really nice.

It mostly involved similar things, but in this case, I ended up performing a query selector for the Select's input to focus on it. The others I made a wrapper for, I could just use the ref provided by the component

https://gist.github.com/ZachHaber/7f443450bb83c92d2c8dbc74dd936f2a

[sholladay]

FYI, with v7 of React Hook Form (which is in beta right now), this situation is going to get slightly worse, in that the usage of refs is sort of implicit and hidden.

With version 7, instead of explicitly passing register into the ref prop, we're supposed to call register, which will return an object of the shape { name, onBlur, onChange, ref } and we're supposed to spread this object into the component.

Here's how it's supposed to work:

import React from 'react';
import { useForm } from 'react-hook-form';
import { Form } from 'semantic-ui-react';

const MyForm = () => {
    const { handleSubmit, register } = useForm();

    return (
        <Form onSubmit={handleSubmit(onSubmit)}>
            <Form.Input
                id="firstName"
                label="First Name"
                {...register('firstName')}
            />
            <Form.Button content="Submit" />
        </Form>
    );
};

But because <Form.Input> doesn't forward the ref to the underlying <input>, this won't work.

It seems like input={register('firstName')} does work, although I'm not clear on whether this is entirely correct to do. For example, that means the name prop will be passed directly to the input instead of via the form field and I'm not sure whether or not the field uses name for anything. Same goes for the event handlers.

[neutraali]

To add to the above, a similar scenario is with <Form.Field>. Using shorthand and trying to pass a ref to the underlying Component won't really pass anything down:

let input = React.useRef();
<Form.Field
	required
	control={CustomInput}
	ref={input}
	label='Required field:'
	name='basic-input'
/>
// input: { current: undefined  }

[brianespinosa]

@neutraali in your example, you would want to attach your ref to your CustomInput component and not the Form.Field wrapper. Even with React.forwardRef you would still need to attach to your input as your ref in your use case.

[sholladay]

@brianespinosa That doesn't seem right. Why couldn't FormField forward the ref to a custom control?

[kyle-sawatsky]

Is there a list anywhere of which components currently forward correctly and which ones need to be updated? Otherwise it's a bit inconvenient to have to try out or look through all the code for each component. The Ref documentation seems to imply that you should just use Ref everywhere for now, for conveniences sake.

[brianespinosa]

Why couldn't FormField forward the ref to a custom control?

@sholladay If I am creating a custom input and place a ref on the FormField and not the custom input... I would be doing that because I explicitly want the ref on the FormField and not the custom input. This is a similar pattern with controlled components. The moment you reach in to component state, we can no longer reliably know what you want to do and you need to control it on your own.

Is there a list anywhere of which components currently forward correctly and which ones need to be updated?

@Hexavolus I do not think this exists. @layershifter was working on another major version bump that would encompass changing the rest of these warnings but it is not done yet. There will be updates here on this ticket when that does get released.

[sholladay]

@Hexavolus I don't think any SUIR components currently implement forwardRef. That's really what this issue is all about, taking the first step.

[kyle-sawatsky]

Ah, my mistake. The documentation said not all components support native ref forwarding which sounded like some did. No worries.

I assume this commonly comes up due to react-hook-form which does provide the Controller component and useController as a workaround for now.