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

Upgrade @reach/router to react-router@6 #14619

Closed
bcomnes opened this issue Apr 15, 2021 · 12 comments · Fixed by samsam-ahmadi/react-trip-date#27
Closed

Upgrade @reach/router to react-router@6 #14619

bcomnes opened this issue Apr 15, 2021 · 12 comments · Fixed by samsam-ahmadi/react-trip-date#27

Comments

@bcomnes
Copy link

bcomnes commented Apr 15, 2021

Storybook uses reach/router in a few critical place places. Reach router is EOL, and unfortunately not receving much attention today (like react 17 support for the time being).

The long term path forward for reach/router is react-router6. The short term path forward is react 17 support for reach.

I attempted to start a PR to port over to react-router6, but unfortunately after a few hours determined its a bit more involved than I am capable with the context I currently posses (subtle changes like navigate coming from a hook instead of an export now, type changes and the use of definitely typed, existing storybook workarounds and wrappers for reach/router etc).

I'm opening this issue to raise awareness of upstream changes that will probably need to be addressed here at some point.

Relavant links:

Reach Router and it’s sibling project React Router are merging as React Router v6. In other words, Reach Router v2 and React Router v6 are the same. There is more information on the maintainers website. --reach.tech/router/

smockle added a commit to primer/react that referenced this issue May 25, 2021
…) has a peer-dep on React 16.x which conflicts with PRC’s peer-dep on React 17.x"

This reverts commit e3612051ab01bf1dfbdf477f00ef96b0ea8aca47.

Storybook has the same issue (storybookjs/storybook#14119, storybookjs/storybook#14619), so if we’re open to using '--legacy-peer-deps' to use Storybook, we can’t justify removing Playroom on anti-'--legacy-peer-deps' grounds.
@cseas
Copy link

cseas commented May 26, 2021

This issue is potentially a bigger problem for users of npm v7 than v6. I get the following error on running npm update in a project with react v17 and storybook:

npm ERR! Could not resolve dependency:
npm ERR! peer react@"15.x || 16.x || 16.4.0-alpha.0911da3" from @reach/router@1.3.4
npm ERR! node_modules/@storybook/api/node_modules/@reach/router
npm ERR! @reach/router@"^1.3.4" from @storybook/api@6.2.9
npm ERR! node_modules/@storybook/api
npm ERR! @storybook/api@"6.2.9" from @storybook/addon-actions@6.2.9
npm ERR! node_modules/@storybook/addon-actions
npm ERR! dev @storybook/addon-actions@"^6.1.17" from the root project
npm ERR! 1 more (@storybook/addon-essentials)
npm ERR! 8 more (@storybook/addon-essentials, @storybook/addons, ...)
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.

reach/router#432 (comment)

@tjjjwxzq
Copy link

tjjjwxzq commented May 27, 2021

Another npm 7 user here, getting the same issue as above. Not sure if things will break if I force an install.

I've had to jump through many hoops just get Storybook working with Gatsby v3, then it recently stopped working again for unknown reasons, and I can't even do a proper clean install now because of this issue.

@IanVS
Copy link
Member

IanVS commented May 27, 2021

I don't think anything will break if you force the install, it will just behave as if you were using npm 6, which only printed warnings.

@kaiyoma
Copy link

kaiyoma commented Jun 23, 2021

I'm running into this same problem as well. I have a Rush mono-repo that uses pnpm as the package manager and I'm trying to upgrade to React 17. pnpm fails to install packages because the peer dependency requirement is not being met:

ERROR  @storybook/addon-actions > @storybook/addons > @storybook/api > @reach/router: create-react-context@0.3.0 requires a peer of react@^0.14.0 || ^15.0.0 || ^16.0.0 but version 17.0.2 was installed.

I don't want to disable this strict-peer-dependency setting for the package manager, because it's a helpful tool to ensure that the packages in the repo are in a sane state. Will this issue be fixed soon?

@IanVS
Copy link
Member

IanVS commented Jun 23, 2021

Unfortunately I think both reach router and react-router aren't getting much love right now as the lead maintainers are heads-down on building a new paid framework, remix. I'm hopeful that they'll come back up for air at some point and give some love to the routers, but for now it seems like we're in a holding pattern. I wonder if the best thing might be to fork reach router (and create-react-context) for now and update the peer-deps. What do you think, @shilman?

@bcomnes
Copy link
Author

bcomnes commented Jun 23, 2021

IMO the best thing to do short term is to do a storybook org fork of reach with react 17 support, and to follow up with that, move back over to react router at the latest version.

I was planning to do the former, but have been slammed at work.

@shilman
Copy link
Member

shilman commented Jun 23, 2021

Seems to me that reach-router is dead. I'd love it if somebody could upgrade to react-router (or a suitable alternative?!) 🙏

@LaurensUP
Copy link

LaurensUP commented Jul 15, 2021

Having issues using storybook with yarn 2 because of this.

...component-repo provides react@npm:17.0.2 with version 17.0.2, which doesn't satisfy the following requirements:

➤ YN0000: @reach/router@npm:1.3.4 [7db54]                → 15.x || 16.x || 16.4.0-alpha.0911da3 ✘
➤ YN0000: @storybook/addon-actions@npm:6.3.4 [a5f0f]     → ^16.8.0 || ^17.0.0                   ✓
➤ YN0000: @storybook/api@npm:6.3.4 [d5c86]               → ^16.8.0 || ^17.0.0                   ✓
➤ YN0000: @storybook/client-api@npm:6.3.4 [d5c86]        → ^16.8.0 || ^17.0.0                   ✓
➤ YN0000: @storybook/components@npm:6.3.4 [d5c86]        → ^16.8.0 || ^17.0.0                   ✓
➤ YN0000: @storybook/router@npm:6.3.4 [7db54]            → ^16.8.0 || ^17.0.0                   ✓
➤ YN0000: @storybook/theming@npm:6.3.4 [d5c86]           → ^16.8.0 || ^17.0.0                   ✓

and

$ yarn why @reach/router
├─ @storybook/api@npm:5.3.21
│  └─ @reach/router@npm:1.3.4 (via npm:^1.2.1)
│
├─ @storybook/api@npm:6.2.9
│  └─ @reach/router@npm:1.3.4 (via npm:^1.3.4)
│
├─ @storybook/api@npm:6.3.4
│  └─ @reach/router@npm:1.3.4 (via npm:^1.3.4)
│
├─ @storybook/api@npm:5.3.21 [25072]
│  └─ @reach/router@npm:1.3.4 [7db54] (via npm:^1.3.4 [7db54])
│
├─ @storybook/api@npm:6.2.9 [ab6be]
│  └─ @reach/router@npm:1.3.4 [7db54] (via npm:^1.3.4 [7db54])
│
├─ @storybook/api@npm:5.3.21 [c440d]
│  └─ @reach/router@npm:1.3.4 [0647a] (via npm:^1.2.1 [0647a])
│
├─ @storybook/api@npm:6.3.4 [d5c86]
│  └─ @reach/router@npm:1.3.4 [7db54] (via npm:^1.3.4 [7db54])
│
├─ @storybook/router@npm:5.3.21
│  └─ @reach/router@npm:1.3.4 (via npm:^1.2.1)
│
├─ @storybook/router@npm:6.2.9
│  └─ @reach/router@npm:1.3.4 (via npm:^1.3.4)
│
├─ @storybook/router@npm:6.3.4
│  └─ @reach/router@npm:1.3.4 (via npm:^1.3.4)
│
├─ @storybook/router@npm:5.3.21 [0647a]
│  └─ @reach/router@npm:1.3.4 [0647a] (via npm:^1.2.1 [0647a])
│
├─ @storybook/router@npm:5.3.21 [25072]
│  └─ @reach/router@npm:1.3.4 [7db54] (via npm:^1.3.4 [7db54])
│
├─ @storybook/router@npm:6.3.4 [7db54]
│  └─ @reach/router@npm:1.3.4 [7db54] (via npm:^1.3.4 [7db54])
│
└─ @storybook/router@npm:6.2.9 [a6752]
   └─ @reach/router@npm:1.3.4 [7db54] (via npm:^1.3.4 [7db54])

Reach-router still depending on a react 16 peerDependency is giving people using Storybook issues.

@ndelangen
Copy link
Member

going to attempt to migrate to react-router

@ndelangen
Copy link
Member

All right, I think I've done, it PR is open. see above. Would be amazing to get some 👀 on it from multiple people!

@shilman
Copy link
Member

shilman commented Oct 25, 2021

w00t!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.4.0-beta.20 containing PR #16440 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Oct 25, 2021
@akuji1993
Copy link

Issue moves on to @storybook/preset-create-react-app version, after installing the Prerelease. So not fixed for React projects, yet.

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

Successfully merging a pull request may close this issue.

9 participants