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

Support for react-router 6.4.0 #95

Open
voodoocreation opened this issue Sep 19, 2022 · 24 comments
Open

Support for react-router 6.4.0 #95

voodoocreation opened this issue Sep 19, 2022 · 24 comments

Comments

@voodoocreation
Copy link

Hey team - was just wondering if there's any planned support for react-router v6.4.0?

After attempting to migrate to their newer method of wiring up routes, it appears it requires a whole new provider to be used, which doesn't seem to require creating the history instance to provide it - I'm assuming it now does it internally. Not too sure what changes would be required within redux-first-router to support this new way of wiring up the router, but at present due to the internal abstraction of history within react-router's new API it doesn't seem to be possible to integrate it with redux-first-history.

Any updates on this would be greatly appreciated 🙏

@salvoravida
Copy link
Owner

wip

@salvoravida
Copy link
Owner

@voodoocreation
Copy link
Author

voodoocreation commented Oct 3, 2022

Hey @salvoravida - yeah the old API still works in >=6.4.0 - it was their newer API using the create...Router methods rather than creating a history instance and passing it down which doesn't seem possible at present. Some more info about that can be found in their docs. For example, using const router = createBrowserRouter(); and then providing router to their RouterProvider component (which is the new way of doing things, which doesn't support history being provided - presumably because the create...Router functions are doing that internally).

Using this newer API is what enables the other new features of [email protected] such as loaders, actions and fetchers.

It's possible to just use the older API and not make use of these new features (which is what I'm currently having to do) but it would be good to provide some tooling that works with their new API as well to be feature-complete (plus I'm not sure what the future of react-router looks like, but it would be very unfortunate if they drop support for the old API in the future).

@voodoocreation
Copy link
Author

You might have to look at the internals of those new methods to see how it's integrating with the history events to see what you'll need to bind to to get the Redux integration working. Fingers crossed it's still possible and isn't something they've obfuscated within those new methods 🤞

@bobsilverberg
Copy link

@voodoocreation it seems working https://codesandbox.io/s/redux-first-history-demo-rr6-forked-8ihzv6?file=/src/App.jsx

Am I missing something?

I am trying this now with react-router-dom v6.4.2, and I'm getting an error from the react-router code after clicking on a Link. I am doing SSR, which might be contributing to it, but up to this point that wasn't an issue. The initial page renders fine, but when I click on a link on the client, I am getting TypeError: locationProp is undefined, which is coming from https://github.com/remix-run/react-router/blob/892238bee052177e78012167c088290b06400206/packages/react-router/lib/components.tsx#L315-L321.

I have tried debugging this and I cannot figure out why location is fine, and defined, at that point when the page is initially rendered on the server, but after clicking a link on the client, location is no longer defined. It seems like when the @@router/LOCATION_CHANGE action is dispatched, the Router itself rerenders, and at that point location is undefined.

If anyone has any insight into what might be happening here, it would be appreciated.

@mogsdad
Copy link

mogsdad commented Nov 4, 2022

I had a config working with [email protected] and just broke with 6.4.3. The TypeScript errors point pretty clearly to a change that added a new encodeLocation property to History in @remix-run/router as a "patch". This divergence presents another challenge for supporting react-router 6.4.

    src/app/components/inputs/__tests__/nav-link.test.tsx:55:20 - error TS2741: Property 'encodeLocation' is missing in type 'History & { listenObject: boolean; }' but required in type 'History'.

    55     <HistoryRouter history={history}>
                          ~~~~~~~

      node_modules/@remix-run/router/dist/history.d.ts:116:5
        116     encodeLocation(location: Location): Location;
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        'encodeLocation' is declared here.
      node_modules/react-router-dom/dist/index.d.ts:51:5
        51     history: History;
               ~~~~~~~
        The expected type comes from property 'history' which is declared here on type 'IntrinsicAttributes & HistoryRouterProps'

@mashood05
Copy link

I had a config working with [email protected] and just broke with 6.4.3. The TypeScript errors point pretty clearly to a change that added a new encodeLocation property to History in @remix-run/router as a "patch". This divergence presents another challenge for supporting react-router 6.4.

    src/app/components/inputs/__tests__/nav-link.test.tsx:55:20 - error TS2741: Property 'encodeLocation' is missing in type 'History & { listenObject: boolean; }' but required in type 'History'.

    55     <HistoryRouter history={history}>
                          ~~~~~~~

      node_modules/@remix-run/router/dist/history.d.ts:116:5
        116     encodeLocation(location: Location): Location;
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        'encodeLocation' is declared here.
      node_modules/react-router-dom/dist/index.d.ts:51:5
        51     history: History;
               ~~~~~~~
        The expected type comes from property 'history' which is declared here on type 'IntrinsicAttributes & HistoryRouterProps'

Hi, are u able to find any solution for this thank u

@bobsilverberg
Copy link

I am still unable to get this to work with the current versions of react-router and redux-first-history. Has anything more been done to try to get this working?

@dabstractor
Copy link

I'm trying to choose a router for all my projects for the next few years and I'm thinking that the lack of activity on this issue makes react-router a deal-breaker if I'm including this package in my stack and want the newer features. Does that sound right? It doesn't seem like a solution is coming after 6 months of activity. Or are you just waiting to see where the dust settles on newer versions of the router?

@elina-codes
Copy link

Any updates on this?

@knownasilya
Copy link

Seems like making your own version of https://github.com/remix-run/react-router/blob/main/packages/react-router-dom/index.tsx#L229 could work, where you allow passing in history?

@kale1d0code
Copy link

kale1d0code commented Oct 5, 2023

I've tried creating my own create...Router from copying the code from createBrowserRouter but passing in history as a curried argument example:

const createHistoryRouter = (history: History) => (
    routes: RouteObject[],
    opts?: DOMRouterOpts
  ): RemixRouter => {
    return createRouter({
      basename: opts?.basename,
      future: {
        ...opts?.future,
        v7_prependBasename: true,
      },
      history: history ?? createBrowserHistory({ window: opts?.window }),
      hydrationData: opts?.hydrationData || parseHydrationData(),
      routes,
      mapRouteProperties,
    }).initialize();
}

The issues I'm facing are.

/

Argument of type 'History' is not assignable to parameter of type 'import("/workspaces/.../node_modules/.pnpm/@remix-run[email protected]/node_modules/@remix-run/router/dist/history").History'.
Type 'History' is missing the following properties from type 'History': action, location, createHref, createURL, and 4 more.
(fixed with //@ts-expect-error)

A call to history.createURL within react-router
which is throwing an error as it states this method doesn't exist.

@kale1d0code
Copy link

kale1d0code commented Oct 6, 2023

I've had a bit more of a go at this by adding the missing functions that the @remix-run/router library expects (dependency of react-router) to the history instance im passing into my createHistoryRouter implementation.

(most functions are copied from @remix-run/router)

example:

import {
  createPath,
  createRouter,
  createBrowserHistory,
  UNSAFE_ErrorResponseImpl as ErrorResponseImpl
} from "@remix-run/router";

import {
  To,
  RouteObject,
  UNSAFE_mapRouteProperties as mapRouteProperties,
} from "react-router";

import type {
  FutureConfig as RouterFutureConfig,
  HydrationState,
  Router as RemixRouter,
  History
} from "@remix-run/router";

declare global {
    var __staticRouterHydrationData: HydrationState | undefined;
}

interface DOMRouterOpts {
    basename?: string;
    future?: Partial<Omit<RouterFutureConfig, "v7_prependBasename">>;
    hydrationData?: HydrationState;
    window?: Window;
}

export function invariant(value: boolean, message?: string): asserts value;
export function invariant<T>(
  value: T | null | undefined,
  message?: string
): asserts value is T;
export function invariant(value: any, message?: string) {
  if (value === false || value === null || typeof value === "undefined") {
    throw new Error(message);
  }
}

function createURL(to: To): URL {
  // window.location.origin is "null" (the literal string value) in Firefox
  // under certain conditions, notably when serving from a local HTML file
  // See https://bugzilla.mozilla.org/show_bug.cgi?id=878297
  let base =
    window.location.origin !== "null"
      ? window.location.origin
      : window.location.href;

  let href = typeof to === "string" ? to : createPath(to);
  invariant(
    base,
    `No window.location.(origin|href) available to create URL for href: ${href}`
  );
  return new URL(href, base)
};

const wrapHistory = (history: History): History => ({
  ...history,
  encodeLocation: (to: To) => {
    // Encode a Location the same way window.location would
    let url = createURL(to);
    return {
      pathname: url.pathname,
      search: url.search,
      hash: url.hash,
    };
  },
  createURL: createURL
});

const createHistoryRouter = (history: History) => (
    routes: RouteObject[],
    opts?: DOMRouterOpts
  ): RemixRouter => {
...
history: wrapHistory(history) ?? createBrowserHistory({ window: opts?.window }),
...
}

however when navigating using a Link component the callstack explodes. checking Redux devtools it looks like it's got stuck in a loop submitting @@router/LOCATION_CHANGE actions.

I looked at the notes for the methods I've added from the History type. I'll post here

(method) History.encodeLocation(to: To): Path
Encode a location the same way window.history would do (no-op for memory history) so we ensure our PUSH/REPLACE navigations for data routers behave the same as POP

@param to — Unencoded path

(method) History.createURL(to: To): URL
Returns a URL for the given to value

@param to — The destination URL

I also fixed this problem with some middleware for redux that watches out for this looping behaviour. I've managed to get a simple demonstration working. I also made sure to add this middleware before the router middleware.

the middleware is as follows:

import {Middleware, PayloadAction} from "@reduxjs/toolkit";

var previousActions: PayloadAction<any>[] = [];

const preventDuplicateLocationChange: Middleware = store => next => action => {
    if ((action.type === "@@router/LOCATION_CHANGE" || action.type === "@@router/CALL_HISTORY_METHOD") && previousActions.length > 1) {
            const currentPath = action.type === "@@router/LOCATION_CHANGE"
            ? action.payload?.location?.pathname
            : action.payload?.args[0]?.pathname;
            const firstPreviousPath = previousActions[0].type === "@@router/LOCATION_CHANGE"
            ? previousActions[0].payload?.location?.pathname
            : previousActions[0].type === "@@router/CALL_HISTORY_METHOD" ? previousActions[0].payload?.args[0]?.pathname : null;
            const SecondPreviousPath = previousActions[1].type === "@@router/LOCATION_CHANGE"
            ? previousActions[1].payload?.location?.pathname
            : previousActions[1].type === "@@router/CALL_HISTORY_METHOD" ? previousActions[1].payload?.args[0]?.pathname : null;
            if (currentPath === firstPreviousPath && currentPath === SecondPreviousPath) {
                return;
            }
    }
    previousActions.unshift(action)
    if (previousActions.length > 2) {
        previousActions.pop()
    }
    return next(action);
}

Although this works, it doesn't actually fix the underlying issue.
the router middleware is dispatching "@@router/CALL_HISTORY_METHOD"
and then "@@router/LOCATION_CHANGE" over and over, when using create...router or @remix-run/router

@CarreraPHP
Copy link

@kale1d0code, Hope you succeed with you attempt of making React router >= 6.4 to work with redux.

But, I ask this question to the entire Redux community out there, Why can't we create or fork the React router V5 and make a Router that has Redux as its integral core logic. I think that would be the permanent solution.

Remix and Nextjs have their plans to support RSC and having not to expose history makes their work easier. The choice that the redux community has is to create a Router that works only through redux and on the client side. For rest of the case, just forget the router and have redux islands that work per page.

@knownasilya
Copy link

knownasilya commented Oct 10, 2023

I'd love to get input from @ryanflorence on recommended approach. I want to use loaders and actions and continue using redux as is, a rewrite would be a huge blocker.

@kale1d0code
Copy link

@kale1d0code, Hope you succeed with you attempt of making React router >= 6.4 to work with redux.

But, I ask this question to the entire Redux community out there, Why can't we create or fork the React router V5 and make a Router that has Redux as its integral core logic. I think that would be the permanent solution.

Remix and Nextjs have their plans to support RSC and having not to expose history makes their work easier. The choice that the redux community has is to create a Router that works only through redux and on the client side. For rest of the case, just forget the router and have redux islands that work per page.

It works like a charm, even got code splitting with routes and reducers working
It what I posted isn't easy enough to follow I'll look into making a gist

@elmarti
Copy link

elmarti commented Nov 28, 2023

I've got RRD V6 working with unstable_HistoryRouter - obviously not ideal and i'll be looking for a long term solution before production, but thought i'd share:

import { unstable_HistoryRouter as HistoryRouter, Routes, Route } from "react-router-dom";
      <HistoryRouter history={history as any}>
 ....

@knownasilya
Copy link

@elmarti using actions and loaders?

@CarreraPHP
Copy link

@elmarti , @kale1d0code , I tried using HistoryRouter as a replacement of BroswerRouter but in that repo, we use connected-react-router package insteadof redux-first-history and it fails. Need to check, how i can migrate to redux-first-history package first.

But, can you guys confirm if history-router from React Router 6.20 is working well with redux-first-history?

@CarreraPHP
Copy link

After replacing connected-react-router with redux-first-history, i am facing the same issue reported by @bobsilverberg as of Oct 26, 2022. The last thing i am going to try is to downgrade to 6.4.2 and check if it works or not.

@voodoocreation it seems working https://codesandbox.io/s/redux-first-history-demo-rr6-forked-8ihzv6?file=/src/App.jsx
Am I missing something?

I am trying this now with react-router-dom v6.4.2, and I'm getting an error from the react-router code after clicking on a Link. I am doing SSR, which might be contributing to it, but up to this point that wasn't an issue. The initial page renders fine, but when I click on a link on the client, I am getting TypeError: locationProp is undefined, which is coming from https://github.com/remix-run/react-router/blob/892238bee052177e78012167c088290b06400206/packages/react-router/lib/components.tsx#L315-L321.

I have tried debugging this and I cannot figure out why location is fine, and defined, at that point when the page is initially rendered on the server, but after clicking a link on the client, location is no longer defined. It seems like when the @@router/LOCATION_CHANGE action is dispatched, the Router itself rerenders, and at that point location is undefined.

If anyone has any insight into what might be happening here, it would be appreciated.

@knownasilya
Copy link

@CarreraPHP 6.2 and probably even 6.4 works, just the loaders and actions do not work because you need a specific router for that.

@kale1d0code
Copy link

@CarreraPHP

I've created the following gist which I hope will help
https://gist.github.com/kale1d0code/6fec7abec7c921d67e7d4463f155d17d

@veeramarni
Copy link

@salvoravida thanks for the nice library. Can you please make this library support 6.4+ officially?

@cha0s
Copy link

cha0s commented Feb 20, 2024

Greetings! You may be interested in my project: https://github.com/cha0s/redux-data-router

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

No branches or pull requests