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

[Bug]: react-router-dom-v5-compat v6.16.0 has an undeclared dependency on @remix-run/router #10904

Closed
jfirebaugh opened this issue Oct 2, 2023 · 2 comments
Labels

Comments

@jfirebaugh
Copy link

What version of React Router are you using?

6.16.0

Steps to Reproduce

pnpm init
echo "hoist=false" >.npmrc
pnpm i react react-dom react-router-dom@5 react-router-dom-v5-compat typescript

tsconfig.json:

{
  "compilerOptions": {
    "lib": ["es2018", "dom"],
  }
}

index.ts:

import { CompatRouter } from "react-router-dom-v5-compat"

Run pnpm tsc.

Expected Behavior

pnpm tsc runs without errors.

Actual Behavior

node_modules/.pnpm/react-router-dom-v5-compat@6.16.0_react-dom@18.2.0_react-router-dom@5.3.4_react@18.2.0/node_modules/react-router-dom-v5-compat/dist/react-router-dom/dom.d.ts:1:71 - error TS2307: Cannot find module '@remix-run/router' or its corresponding type declarations.

1 import type { FormEncType, HTMLFormMethod, RelativeRoutingType } from "@remix-run/router";
                                                                        ~~~~~~~~~~~~~~~~~~~

node_modules/.pnpm/react-router-dom-v5-compat@6.16.0_react-dom@18.2.0_react-router-dom@5.3.4_react@18.2.0/node_modules/react-router-dom-v5-compat/dist/react-router-dom/index.d.ts:7:203 - error TS2307: Cannot find module '@remix-run/router' or its corresponding type declarations.

7 import type { Fetcher, FormEncType, FormMethod, FutureConfig as RouterFutureConfig, GetScrollRestorationKeyFunction, History, HTMLFormMethod, HydrationState, Router as RemixRouter, V7_FormMethod } from "@remix-run/router";
                                                                                                                                                                                                            ~~~~~~~~~~~~~~~~~~~


Found 2 errors in 2 files.
@jfirebaugh jfirebaugh added the bug label Oct 2, 2023
@timdorr
Copy link
Member

timdorr commented Oct 2, 2023

This is something with pnpm. react-router-dom-v5-compat depends on react-router@6, which depends on @remix-run/router. The dependency is definitely installed and will be visible with npm ls @remix-run/router

@timdorr timdorr closed this as completed Oct 2, 2023
@jfirebaugh
Copy link
Author

jfirebaugh commented Oct 2, 2023

It's something that happens to work with npm's hoisting algorithm, but does not work with pnpm with hoist=false or with Yarn PnP (see "Ghost dependencies protection"), and may not even work with npm in the future depending on the outcome of npm/rfcs#287.

Packages should always declare dependencies for packages they directly require or import, and not rely on transitive dependencies getting hoisted.

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

No branches or pull requests

2 participants