Support .attrs({ ref: ... })

[simonbuchan]

Support using ref inside .attrs() argument, to dynamically generate a ref.

Basically the goal is implementing tab order like behavior in react-native:

const refList = [React.createRef(), React.createRef()]
const handleFocus = refList.map(ref => () => ref.current.focus())
const common = { refList, handleFocus, ... }

<MyInput {...common} refIndex={0} />
<MyInput {...common} refIndex={1} />

const MyInput = styled.TextInput.attrs({
  ref: props => props.refList[props.refIndex],
  onSubmitEditing: props => props.handleFocus[props.refIndex + 1],
})

In theory key could be supported in the same way, but that can be a separate PR if wanted.

[mxstbr-bot]

	Warnings
⚠️	There are library changes, but not tests. That's OK as long as you're refactoring existing code

Generated by 🚫 dangerJS

[kitten]

I think this is a great idea, but it potentially conflicts with what innerRef is. We pass innerRef on unchanged instead of creating a ref if the wrapped component is a StyledComponent as well.

This subtle feature would be a little confusing by calling the attribute ref. cc @probablyup @mxstbr: I think this is great, but shall we call the attr innerRef as well?

[simonbuchan]

Well the problem is the directly wrapped element gets either key or innerKey depending on its type. Otherwise I'm fine with either.
One thing I ran into is styled(Inner) will stomp the attrs ref of Inner if it's also a styled component in react native - Inner.extend seems fine. Not sure what to do about that other than have another ref so Inner can tell this isn't from the user?

[simonbuchan]

Ah, the ref error is enzymejs/enzyme#1504
Easy enough to work around.

[simonbuchan]

Description cleaned up, rebased and squashed, tests working (was actually just react-dom needed to be updated and the errors were being swallowed!?)

@kitten I'm going to stick with ref for now as @probablyup implied in #1718 4.0 will move to using forwardRef for styled components, which would make/allow the outer element usage ref instead of innerRef as well.

The confusion is basically over whether you feel attrs() is closer to defaultProps (should match outer attributes) or the render() implementation (should match what the wrapped component receives) - I've gone with the latter here - especially since attrs() can have logic based on the outer props.

[quantizor]

Alright so I looked into this for v4 and I don't think it's possible without adding another component layer above due to how forwardRef works. If you want to do that for your project, that's fine and it'll probably work but we wouldn't want to add it to core due to the perf impact for everyone who isn't using that functionality.

[simonbuchan]

Fair enough. I haven't looked into it with v4, but I suppose you could also keep the current innerRef implementation as well and pass-through .attrs({ ref }) that way, but it's a bit overkill.

When you say "another component layer above", you mean a raw component, not a styled component; something like this?

const A = styled.input`color: red`;
const B = (props) => (
  <A
    {...props}
    ref={props.refList[props.refIndex]}
    onKeyPress={props.handleFocus[props.refIndex + 1]}
  />
);

It always feels like I'm doing something wrong when I do that, but I suppose it can't be helped here.

[quantizor]

Well so the issue with forwardRef is the ref={} prop has to be put on the uppermost level for it to be forwarded. attrs is applied one level down in the StyledComponent class, so ref forwarding wouldn't work at that point.

[simonbuchan]

Sure, but the inner component also has access to the real component to give to ref, forwardRef() just gives you a way to get the literal ref parameter passed.

[quantizor]

I mean I tried redoing your PR against develop and it just didn't work with forwardRef. You're welcome to give it another try though if you can get it working and not regress performance.