1.6.1 is breaking

[AlicanC]

I am aware that usage of React.forwardRef is listed under "Potentially Breaking" in the change log. It says:
This is a breaking change for people relying on the relayContainer.refs.component implementation detail.

Sadly, there is more to it. Our app is completely broken, because we have cases like this:

class MyComponent extends Component {
  // ...
}

const MyComponentPaginationContainer = createPaginationContainer(
  MyComponent,
  // ...
);

export default connect(
  // ...
)(MyComponentPaginationContainer);

After upgrading to 1.6.2, connect()s like this started throwing:
You must pass a component to the function returned by connect. Instead received {}

We do this because we need data from the Redux store that we use in query variables so we don't have the option to re-order createPaginationContainer() and connect().

I don't know if we are benefiting from internal usage of it, but other than that we really don't care about React.forwardRef, but we need other changes in 1.6.1.

Official React documents also state that usage of React.forwardRef should bump major:
https://reactjs.org/docs/forwarding-refs.html#note-for-component-library-maintainers

Could this be respected?

[AndreiCalazans]

I know it's weird but have you tried wrapping MyComponent with the connect HOC before passing to createPaginationContainer?

like

class MyComponent extends Component {
  // ...
}

const Connected = connect(...)(MyComponent);

const MyComponentPaginationContainer = createPaginationContainer(
  Connected,
  // ...
);

export default MyComponentPaginationContainer;

Although this is just a workaround

[sibelius]

This looks like an issue with React Redux instead

Check this tweet https://twitter.com/acemarke/status/1024761710765334528?s=21

ForwardRef does not work well with React Redux

[AlicanC]

@AndreiCalazans

I can't do that. As I've said:

We do this because we need data from the Redux store that we use in query variables so we don't have the option to re-order createPaginationContainer() and connect().

There are other ways but I won't be refactoring tons of components with workarounds for this. I'd rather wait for stuff to get fixed.

@sibelius I am aware that Redux can't handle forwardRef.

I am just saying that:

• As stated in the official React documentation, using forwardRefs is a breaking change.
• A breaking change was made in Relay.
• This breaking change was released without bumping major.
• The release broke our application.

Relay 1.6.1 clearly violates SemVer and I see two options here:

• Take no action which would mean Relay does not follow SemVer.
• Rollback the change

[sibelius]

1.6.0 was a breaking change as well, as it set the minimum React version to 16.4.x

[cyan33]

We fixed this in Facebook by forking react-redux and removed that invariant error. Hopefully it can be fixed upstream soon. cc @markerikson :)

[markerikson]

I don't think this is about connect using forwardRef. It's about our check for valid components being passed in, which is a separate open issue about our use of react-is.

[cyan33]

@markerikson Yeah sorry I wasn't making it clear. That's the exactly the issue I'm talking about. Tracker: reduxjs/react-redux#914.

[kassens]

I'll try to push us to be more careful about bumping the version correctly, but keeping track of the breaking changes can sometimes be hard and slip through.

One thing that made this worse is that we've had a long lag without any releases increasing the likelihood of something slipping through.

We've done some internal improvements to make it easier to release and hopefully can do that just more regularly to keep the list of changes down to fewer commits.

I don't think there's much left here as we can't revert to the old ref API as React.forwardRef allows us to move forward with the next improvements!

[nodkz]

@kassens yeah, more often releases will be a good thing for the community. One release in half of the year looks so like that the project is unmaintained well.

Minimal one release per a month will be a good deal 😎

[kassens]

@nodkz agreed! Just finished a new RC https://github.com/facebook/relay/releases/tag/v1.7.0-rc.1
It's just a question of keeping the momentum.

[nodkz]

Rock it! Take care about OSS Relay package, who else if not you!

[tlhunter]

FWIW the only change from 1.6.1 to 1.6.2 was a minor dependency update:
https://diff.intrinsic.com/react-relay/1.6.1/1.6.2#file-package.json