react: `typeof React.forwardRef(...)` is not a 'function', breaking third-party libs

If my-component-library depends on a new major version of React, shouldn’t my-component-library receive a major version bump as well?

Sure, but you can make the same argument about my-component-library starting to depend on a new feature that was introduced in a minor version of React, results in an observably different behavior, and didn’t exist before. Which is exactly my point: relying on a new feature is a breaking change whereas adding a new feature is not.

I don’t understand what you’re suggesting. forwardRef() doesn’t accept a component at all (it accepts a rendering function). There are good reasons why the returned type is not a function (see last paragraph of https://github.com/reduxjs/react-redux/issues/914#issuecomment-396410016). We don’t plan to cut a patch to change this.

Again, a library adopting forwardRef should constitute a major bump for that library. Even if React did something magical to ensure forwardRef return value is a function it still has a different observable behavior (namely, it changes what a ref points to, and depends on a method that does not exist in older versions), so a library would have to bump its major version anyway.

This new method has been added to react-is and will go out with the upcoming release. Thanks!

It’s unintuitive to me because I would expect React to throw in the createElement() call if it received an invalid element type (even though, as of today, it appears that that check is deferred until the renderer begins mounting the element).

It intentionally doesn’t throw so that we can easily inline this function or replace it with a compiler output.

there’s compelling value to being able to check at HOC instantiation time that the component you’re wrapping in the HOC is a valid component type

I agree this is useful to HOCs.

We will be releasing the package Dan mentioned (react-is) along with 16.3. It exposes a function called isForwardRef, which could be used here. Although this function expects React elements (the return type of React.createElement) rather than a naked Component type, as the next.js validation check seems to be expecting.

Put another way:

const { isForwardRef } = require("react-is");
isForwardRef(React.createElement(Component)); // true
isForwardRef(<Component />); // true
isForwardRef(Component); // false

This is one of those “don’t break the internet” bugs.

As an aside, I’d prefer if we reserved this terminology for its original meaning: “ship something that breaks existing websites without any direct action on their behalf”. For example, Chrome’s “interventions”, Array.prototype.contains, etc.

Using this phrase to refer to a change where you opt in (by bumping the version of React, and later bumping some library to a new major—after all, it is now 16.3+—relying on forwardRef in public API) doesn’t seem entirely fair to me. It makes “break the internet” sound cheaper and more commonplace than it is.

@sanfilippopablo I don’t see why this issue is specific to forwardRef. We’ve always been very explicit about inheritance not being the best method for code reuse in React. It already doesn’t work with functional components. You can think of forwardRef as something being close in spirit to a functional component, and in that sense it makes sense that it doesn’t work.

another change this causes is the inability to use scryRenderedComponentsWithType and similar methods that search the tree for composite components

Mind filing a separate issue so I can take a look? That sounds accidental to me.

Fair enough. I was just pointing out a difference between what Next is doing and how react-is works.

React does not expose a utility to check whether a given value is valid to be passed to React.createElement

I don’t think we’re opposed to exposing a utility like this. We’re already releasing a package called react-is with some brand checking utilities along with 16.3, and I think we could add something like isValidElementType to it.

it may be a good idea to change forwardRef to return a ‘function’, instead of an object.

Then we’d have to look for a “special” field on every single function/class pass to React. I don’t think we’d want to go with that strategy since it introduces an extra cost for the majority of cases.

Ultimately I don’t think this problem is a blocker. It’s unfortunate, but React widening supported types is not a semver major change. If as a library author you choose to depend on the fact that React only supports defining components as classes or functions (and not, e.g., with objects), you are kind of committing to closely tracking what exactly it is that React supports. Of course a better solution would be to have a way to check it that’s maintained by us—and as I said, I think we’re open to providing that.