-
Notifications
You must be signed in to change notification settings - Fork 30
Ask people to use 5.1 instead? #24
Comments
Hi @mjackson. Thanks for your great work! While I encourage people to use the first-party hooks, I don't want to deprecate this for users who are unable or unwilling to upgrade to 5.1 or refactor their existing code. A helpful balance may be to set the I'll leave this issue open until I think of the optimal solution for users, and I encourage readers to migrate going forward. |
No better way to encourage people than by deprecating your package :) They'll still be able to use it. If you don't want to deprecate, would you at least please put a link in the README to this issue as a "known issue" so that people know there are risks involved with using this package? |
Hi @CharlesStover 馃憢
I think this package has been causing some problems with people who are using React Router and getting 2 different copies of our context object. The problem is that you depend on
react-router
, so you have one instance of our__RouterContext
object. Butreact-router-dom
andreact-router-native
also depend onreact-router
, so if the 2 copies don't match up exactly then people end up using a different instance of context which breaks things.It's hard to reproduce this behavior because package managers vary in how they decide to hoist dependencies and deduplicate them. But we've seen a few issues crop up in the main router repo that seem to indicate we have an ecosystem problem (see remix-run/react-router#6755).
The solution is that you should never depend on
react-router
directly. Instead, you should depend onreact-router-dom
orreact-router-native
, depending on which platform you're targeting.But there's also some good news, and that is that we shipped some hooks yesterday in v5.1, so people shouldn't need
use-react-router
anymore. So an even simpler solution would be to deprecate this package with a notice message that encourages people to use v5.1 instead. This would also be ideal for us since we never intended 3rd party libs to use our context object (hence the__
prefix).Thanks!
The text was updated successfully, but these errors were encountered: