3

TL;DR: I have a function component in a library that has react@^16.3.0 as a peer dependency. Is it a breaking change if I wrap this component in React.forwardRef?

According to the react docs in forwarding Refs#Note for component library maintainers introducing ref forwarding to your components is a breaking change.

I understand this for class components since before you would get a ref to the class and now you would get a ref to whatever you forward this to.

However for function components refs were never supported in the first place and would trigger warnings.

Is it safe to assume that introducing forwardRef to function components is a feature and not a breaking change?

For me this is similar to adding a new property to your API. Before I would get type errors whereas now I get a defined value. Overall this sounds like the docs state that the non-existence of a feature is actually a feature which would mean that every feature is a breaking change.

Edit: React is already required as a peer with ^16.3.0 so we would already have access to forwardRef. We would not require users to upgrade their react version.

Sebastian
  • 3,322
  • 22
  • 34
  • 1
    I think you are right, in this specific case this is only a new feature you were not able to use before, so I don't see dangers here – keul Nov 23 '18 at 13:24

2 Answers2

1

forwardRef may provide breaking changes for library users regardless of component type because this depends on public API a library provides in the first place:

When you start using forwardRef in a component library, you should treat it as a breaking change and release a new major version of your library. This is because your library likely has an observably different behavior (such as what refs get assigned to, and what types are exported)

Previously a ref was expected to be either DOM element or class instance. Now a ref can potentially be something else. If class component

// can be used like <Foo ref={this.foo}/> and this.foo.doFoo()
class Foo extends Component {
  doFoo() {}
  ...
}

is replaced with

const Foo = forwardRef((props, ref) => {
  ref.current = { doFoo: () => {} };
  return ...;
});

public API, this may introduce breaking change in case a user already considers it class instance, e.g. in tests.

Another scenario is functional component that could be called directly like suggested here to provide a ref to its child. It's a hack which will break if functional component is replaced with forwardRef.

Another thing is that forwardRef is available since 16.3 and cannot be efficiently polyfilled. React version requirement may also introduce breaking change.

Estus Flask
  • 206,104
  • 70
  • 425
  • 565
  • 1
    Previously a ref was expected to be either DOM node, class instance or none for function components. This is important because I'm specifically targeting function components. We already require react@^16.3.0. I forgot to mention that. – Sebastian Nov 23 '18 at 15:07
  • I see. It's unclear from the question whether your case is a library or your own application. The remark you linked applies only to libraries because refs in their API may change without notice. – Estus Flask Nov 23 '18 at 15:14
  • but since component is functional there are no users relying on `ref`. so there is no code possible to be broken. – skyboyer Nov 23 '18 at 15:15
  • 1
    @skyboyer It may be broken if it previously wasn't functional and now it's functional. Another issue I see if functional component was called directly to get a ref. – Estus Flask Nov 23 '18 at 15:19
  • Ok if it was not apparent from the link that targets library maintainers: This is a function component in a library to which I want to introduce a breaking change. The react docs say that forwardRef is always a breaking change but I'm having a hard time imagining that users rely on the non-existence of a feature – Sebastian Nov 23 '18 at 15:21
  • @epsilon This applies to your own function components only if you used workarounds like passing something like `internalRefProp` and forgot to rewrite your own comps to use `ref` instead of `internalRefProp` when switching to `forwardRef`. – Estus Flask Nov 23 '18 at 15:31
1

One of the issues I encountered myself is that hoist-non-react-statics required a major version bump to support React.forwardRef. Everyone using hoist-non-react-statics@2.x (popular for higher-order components) would need to update that as well because an enhanced component that was also decorated with hoistNonReactStatics would cause ReactIs.isForwardRef to return true even if the enhanced component is a function component.

Sebastian
  • 3,322
  • 22
  • 34