Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Impossible to create a lib compatible with version 7 and 8 #1914

Closed
1 task done
paztis opened this issue Apr 28, 2022 · 5 comments · Fixed by #1922
Closed
1 task done

Impossible to create a lib compatible with version 7 and 8 #1914

paztis opened this issue Apr 28, 2022 · 5 comments · Fixed by #1922

Comments

@paztis
Copy link

paztis commented Apr 28, 2022

What version of React, ReactDOM/React Native, Redux, and React Redux are you using?

  • React: 18
  • ReactDOM: 18
  • Redux: 4.2
  • React Redux: 7 or 8

What is the current behavior?

When we want to create a lib with a peer dependency to react-redux we face a problem.
People that use react 18 will provide react-redux 8. People that work with react 17 will provide react-redux 7.

The lib is doing basic things, but it uses the connect with pure option to true.
In v8, pure option is no more necessary but it throws an error to say the option is not necessary.
This exception makes the full app crash.
It is not possible to remove the option to let v7 correctly works.

We have this case in an unmaintained lib react-beautifull-dnd that blocks the migration of apps to react 18.

What is the expected behavior?

A console warning can be displayed but not a blocking error, mostly if this parameter is not used at all.

Which browser and OS are affected by this issue?

All

Did this work in previous versions of React Redux?

  • Yes
@markerikson
Copy link
Contributor

markerikson commented Apr 28, 2022

Hmm. My first answer here is that you shouldn't have to be passing in pure: true in the first place even in v7, because that's already the default and has been for years.

Oh, wait... you're saying it's not your lib, it's in someone else's lib:

https://github.com/atlassian/react-beautiful-dnd/blob/2360665305b854434e968e41c7b4105009b73c40/src/view/droppable/connected-droppable.js#L270

sigh

That's... silly.

Yeah, changing this to be a one-time-only warning seems reasonable (similar to how React internally tracks "have we shown this warning once?"):

https://github.com/facebook/react/blob/d5f1b067c8bbb826b823d0354a28ba31078b70c0/packages/react/src/ReactContext.js#L44-L65

Afraid this isn't my highest priority atm, but if someone would like to submit a PR we can put that in.

@paztis
Copy link
Author

paztis commented Apr 28, 2022

The problem is react-beautifull-dnd is unmaintained since 1 year but still really used. This one is blocking our migration to react 18 / react-redux 8.
Changing from error to warning will unlock the problem.

More the error is only thrown in dev mode. So it can be good to only warn not ?

@leosuncin
Copy link

@paztis there's a fork of react-beautiful-dnd with fix in progress, maybe the fork is an more likely alternative

@paztis
Copy link
Author

paztis commented Apr 29, 2022

Thanks ! Will look at it

@markerikson
Copy link
Contributor

Just published https://github.com/reduxjs/react-redux/releases/tag/v8.0.2 , which should fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants