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

v7 - types not being exported by react-router #12431

Closed
kelvin2200 opened this issue Dec 1, 2024 · 15 comments
Closed

v7 - types not being exported by react-router #12431

kelvin2200 opened this issue Dec 1, 2024 · 15 comments

Comments

@kelvin2200
Copy link

kelvin2200 commented Dec 1, 2024

I'm using React Router as a...

library

Reproduction

import { matchRoutes } from "react-router";

import { createBrowserHistory } from "history";


export const history = createBrowserHistory();

const routes = [];

/**
 * Bogus function
 * not used anywhere
 * needed to generate a type that we cannot import
 */
function makeInitialMatchesDummy(routes: any[]) {
	return matchRoutes(routes, history.location.pathname);
}

const initialMatches: NonNullable<ReturnType<typeof makeInitialMatchesDummy>> = [];

// exporting this type results in an error (does not happen with v6)
export type IniMatches = typeof initialMatches;

System Info

Windows 11, Vite, Yarn 4.5.3

Used Package Manager

yarn

Expected Behavior

I expect to be able to re-export any type that I can infer

Actual Behavior

react-router-7

@kelvin2200 kelvin2200 added the bug label Dec 1, 2024
@NicoLaval
Copy link

NicoLaval commented Dec 2, 2024

Same here, temporarily import "react-router/dist/production" instead of "react-router" to have types. But it breaks running...

@brookslybrand
Copy link
Contributor

@kelvin2200

I'm not able to reproduce this. I'm wondering if it's something specific to your tsconfig file. Can you please provide a reproduction on Stackblitz or something similar?

@cjnoname
Copy link

Hi @brookslybrand ,

I hope the functions/types below can be exported from react-router as we are using them in our current code.

import { History, Listener } from "@remix-run/router/history";
import { createMemoryHistory, createBrowserHistory } from "@remix-run/router";

@brookslybrand
Copy link
Contributor

These all come from history (a dependency of react-router).

I'm not sure why we were re-exporting them before/why we stopped now (maybe @brophdawg11 has some context)

I'm a little skeptical you need these though if you are already using react-router. These should be implementation details, but of course I don't know the context of what you're building

@cjnoname
Copy link

These all come from history (a dependency of react-router).

I'm not sure why we were re-exporting them before/why we stopped now (maybe @brophdawg11 has some context)

I'm a little skeptical you need these though if you are already using react-router. These should be implementation details, but of course I don't know the context of what you're building

Hey mate,

We use it for module federation. I tried importing these features directly from the history package, but it is not included as part of react-router v7

import type { Theme } from "@mui/material";
import type { History, Listener } from "@remix-run/router/history";
import { createMemoryHistory, createBrowserHistory } from "@remix-run/router";
import { createRoot } from "react-dom/client";
import App from "./App";
import { wsClient } from "./config/apollo/wsClient";
import type { User } from "./graphql/graphqlTypes";

export interface IParentProps {
  onParentNavigate?: Listener;
  defaultHistory: History;
  initialPath?: string;
  user?: User;
  theme?: Theme;
}

const mount = (
  elem: any,
  { onParentNavigate, defaultHistory, initialPath, ...rest }: IParentProps
) => {
  const childHistory =
    defaultHistory ||
    createMemoryHistory({
      initialEntries: [initialPath!]
    });

  if (onParentNavigate) {
    // update child history when parent history changes
    childHistory.listen(onParentNavigate);
  }

  // reset websocket
  wsClient.resetStore();
  const root = createRoot(elem);
  root.render(<App {...rest} />);

  return {
    // give parent a handle, when child history changes, update the parent history
    onChildNavigate: (currentHistory: History) => {
      const nextPathname = currentHistory.location.pathname;
      const currentPathname = childHistory.location.pathname;
      if (nextPathname !== currentPathname) {
        childHistory.push(nextPathname);
      }
    }
  };
};
export { mount };

if (process.env.NODE_ENV === "development") {
  const devRoot = document.querySelector("#child-root");
  if (devRoot) {
    mount(devRoot, {
      defaultHistory: createBrowserHistory()
    });
  }
}

@brophdawg11
Copy link
Contributor

@kelvin2200 Can you provide a reproduction of your issue? I'm also unable to reproduce it.

One potential issue is that the history package is no longer used as of react-router 6.4 when we inlined the history implementations as an implementation detail because the data-aware router's are now the entry point for navigations. A secondary reason is to move away from window.history in preparation for moving the internals to the Navigation API in the future.

@cjnoname The deep imports from @remix-run/router/history were not official public API, and we've moved to using package.json exports in v7 to tighten up the public API surface and prevent the usage of deep imports such as this.

I think we stopped exporting the createBrowserHistory/etc methods in v7 because they're intended to be implementation details of the new createBrowserRouter alternatives. I think we still export UNSAFE_createBrowserHistory and I could probably be convinced to do the same for the other 2 histories - but I don't think we want to encourage continued usage of raw history instances so I don't think we want those to be normal public API (hence the UNSAFE_ prefix).

As for module federation usages, we released the patchRoutesOnNavigation API in v6 which helps some of those setups, so unsure if that's helpful to you. Here's a few references worth checking out

@cjnoname
Copy link

@kelvin2200 Can you provide a reproduction of your issue? I'm also unable to reproduce it.

One potential issue is that the history package is no longer used as of react-router 6.4 when we inlined the history implementations as an implementation detail because the data-aware router's are now the entry point for navigations. A secondary reason is to move away from window.history in preparation for moving the internals to the Navigation API in the future.

@cjnoname The deep imports from @remix-run/router/history were not official public API, and we've moved to using package.json exports in v7 to tighten up the public API surface and prevent the usage of deep imports such as this.

I think we stopped exporting the createBrowserHistory/etc methods in v7 because they're intended to be implementation details of the new createBrowserRouter alternatives. I think we still export UNSAFE_createBrowserHistory and I could probably be convinced to do the same for the other 2 histories - but I don't think we want to encourage continued usage of raw history instances so I don't think we want those to be normal public API (hence the UNSAFE_ prefix).

As for module federation usages, we released the patchRoutesOnNavigation API in v6 which helps some of those setups, so unsure if that's helpful to you. Here's a few references worth checking out

Hi @brophdawg11,

Thanks for helping to export UNSAFE_ functions. I will read through the documents you shared.

@kelvin2200
Copy link
Author

@brophdawg11 the reproduction is in the code I posted.
I don't think you understand the problem, because it has nothing to do with the history implementation used.
Or maybe it does and I don't understand...
Me and probably others need the AgnosticRouteMatch type which is not being exported anywhere in react-router.

What I did in the past, was to import matchRoutes and infer AgnosticRouteMatch from its output, but that no longer works.

I am using react-router in a minimal form, in an implementation with react-relay for render-as-you-fetch patterns and preloading queries.

Furthermore, (even though this may be off-topic)... WHY did you guys turn this into a (yet another) framework?
Where's the benefit? Most of us just need a push/replace method or some basic routing stuff. Now we have to drag along an entire framework in our projects?
Cause at the moment I can only see the downsides. Some docs, examples, reasoning of the entire philosophy?
How/why should devs adopt this?

@Noyabronok
Copy link

As a workaround for AgnosticRouteMatch<string, DataRouteObject>[] | null
you can do ReturnType<typeof matchRoutes>
matchRoutes can be imported from react-router

@brophdawg11
Copy link
Contributor

Apologies for the delay - I was on paternity leave at the end of the year so wasn't able to keep up with github notifications - and only say today's comment in my inbox.

@brophdawg11 the reproduction is in the code I posted.

As both Brooks and I said, we cannot reproduce it from the code you provided so we will need a working reproduction to dig further. From the error screenshot it seems like it might have something to do with your yarn setup?

I don't think you understand the problem, because it has nothing to do with the history implementation used.

Your reproduction code is using history so I didn't want to rule that out since that should no longer be used in current RR implementations.

WHY did you guys turn this into a (yet another) framework?

We didn't. We merged an existing framework (Remix) into this repository and made it available in React Router v7 as a new and completely optional approach. React Router in library mode is effectively the same as v6.

How/why should devs adopt this?

We've outlined the features the vite plugin (framework mode) provides in the upgrade guider: https://reactrouter.com/upgrading/component-routes#features

@jdnichollsc
Copy link

Hello folks, is there any way to use matchRoutes utility with the array of routes returned from the app/routes.ts file while using Framework mode?

Copy link
Contributor

github-actions bot commented Apr 1, 2025

This issue has been automatically closed because we haven't received a response from the original author 🙈. This automation helps keep the issue tracker clean from issues that aren't actionable. Please reach out if you have more information for us! 🙂

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 1, 2025
@kelvin2200
Copy link
Author

@brophdawg11

I am trying to infer a type that is explicitly not being exported by the lib.

The TS error makes sense and it should happen regardless of the package manager used.

I patched the lib, exported that type, and the error went away.

May I ask, why is this so controversial?

Why can't u guys just export the damn type. It seems I'm not the only one in need of it.

FFs....it's just a type.

@brophdawg11
Copy link
Contributor

Nothing here is controversial. We asked for a reproduction, didn't receive one, and eventually our bot closed the issue due to the lack of a reproduction.

Why can't u guys just export the damn type

This type of tone isn't going to win you any favors 🙃

@jdnichollsc
Copy link

initial workaround for server side code:

import { RouteConfigEntry } from "@react-router/dev/routes";
import { matchRoutes } from "react-router";
import routes from "@/routes"; // importing your routes file here

// Extract the type from matchRoutes function parameter
type AgnosticRouteObject = Parameters<typeof matchRoutes>[0][0];

export function convertRouteConfig(entries: RouteConfigEntry[]): AgnosticRouteObject[] {
  return entries.map((entry) => {
    const routeObj: AgnosticRouteObject = {
      index: entry.index,
      path: entry.path,
      caseSensitive: entry.caseSensitive,
      id: entry.id,
    };
    if (entry.children) {
      routeObj.children = convertRouteConfig(entry.children);
    }
    return routeObj;
  });
}

...
const matches = matchRoutes(convertRouteConfig(routes), history.location.pathname);

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

No branches or pull requests

8 participants