Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Ask people to use 5.1 instead? #24

Open
mjackson opened this issue Sep 25, 2019 · 2 comments
Open

Ask people to use 5.1 instead? #24

mjackson opened this issue Sep 25, 2019 · 2 comments

Comments

@mjackson
Copy link

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. But react-router-dom and react-router-native also depend on react-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 on react-router-dom or react-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!

@quisido
Copy link
Collaborator

quisido commented Sep 25, 2019

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 peerDependency to ~5.0.0 instead of ^5.0.0 and have this repo's documentation encourage users to read the react-router's hooks documentation as a preferred alternative.

I'll leave this issue open until I think of the optimal solution for users, and I encourage readers to migrate going forward.

@mjackson
Copy link
Author

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. npm just lets them know at install time that they should be using something else. But they are free to ignore the warning and use it anyway.

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?

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

No branches or pull requests

2 participants