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

React-router v6 TypeScript issues #196

Closed
jtheberge opened this issue Nov 23, 2021 · 18 comments
Closed

React-router v6 TypeScript issues #196

jtheberge opened this issue Nov 23, 2021 · 18 comments

Comments

@jtheberge
Copy link

Referencing set-up from https://github.com/pbeshai/use-query-params/blob/master/examples/react-router-6/src/index.js, I'm getting some TypeScript issues. For example, children is invoked as a function rather than treated as ReactNode. Refer to screenshot:
Screen Shot 2021-11-22 at 5 55 12 PM

@jtheberge
Copy link
Author

jtheberge commented Nov 23, 2021

For location fix, I added

import { Location } from 'history';
...
const RouteAdapter:React.FC = ({ children }) => {
  const navigate = useNavigate();
  const location = useLocation();

  const adaptedHistory = useMemo(
    () => ({
      // can disable eslint for parts here, location.state can be anything
      replace(location: Location) {
        // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
        reactRouterNavigate(location, { replace: true, state: location.state });
      },
      push(location: Location) {
        reactRouterNavigate(location, {
          replace: false,
          // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
          state: location.state,
        });
      },
    }),
    [reactRouterNavigate]
  );
  // https://github.com/pbeshai/use-query-params/issues/196
  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
  // @ts-ignore
  // eslint-disable-next-line @typescript-eslint/no-unsafe-return
  return children({ history: adaptedHistory, reactRouterLocation });
};

// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment had to be added since location.state is any - relevant: remix-run/history#903

Also additional note, I get in console:
index.tsx:25 You should call navigate() in a React.useEffect(), not when your component is first rendered.
Though that looks like a separate issue.

@jacobgavin
Copy link

Until this lib gets react router v6 support (maybe a complete re-write is better) I created this hook #108 (comment) which can be used as useQueryParam. I guess building one that mimics useQueryParams could be done in a similar manner.

@pbeshai
Copy link
Owner

pbeshai commented Nov 30, 2021

Your children type error is likely due to you casting RouteAdapter as React.FC, which shouldn't be necessary, but thank you for sharing this example!

@jtheberge
Copy link
Author

Hey @pbeshai! I think we need to cast it because of

ReactRouterRoute?: React.ComponentClass | React.FunctionComponent;
. It seems to need to be a React functional or class component.

@ryan-williams
Copy link

Thanks so much @jtheberge, your code above including the error disabling on the children call did the trick.

@pbeshai if you want to edit this comment to link here that might be good, I started with that but then got stuck trying to typescript-ify it a few ways before I looked again at the above

@optimistic-updt
Copy link

@jtheberge
Thank you so much for your contribution. saved me so much time!
but just FYI the above snippet code was broken due to using variables that weren't defined ( reactRouterNavigate, reactRouterLocation).
Updated got below

const RouteAdapter: React.FC = ({ children }) => {
  const reactRouterNavigate = useNavigate();
  const reactRouterLocation = useLocation();

  const adaptedHistory = useMemo(
    () => ({
      // can disable eslint for parts here, location.state can be anything
      replace(location: Location) {
        // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
        reactRouterNavigate(location, { replace: true, state: location.state });
      },
      push(location: Location) {
        reactRouterNavigate(location, {
          replace: false,
          // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
          state: location.state,
        });
      },
    }),
    [reactRouterNavigate]
  );
  // https://github.com/pbeshai/use-query-params/issues/196
  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
  // @ts-ignore
  // eslint-disable-next-line @typescript-eslint/no-unsafe-return
  return children({ history: adaptedHistory, reactRouterLocation });
};

Thank you so much 🙏

@moroale93
Copy link

If you look at the compiled and minimized JS code, you will see that the type React.ComponentClass | React.FunctionComponent is incorrect for the ReactRouterRoute property because the children can't be a React.ReactNode type. The QueryParamProviderProps interface is a not generic type, so the typings will never match without forcing them.
The only thing you can do here is to add as many type checks as you can and try to reduce the impact you will have on maintaining your code.
I suggest your to use this:

import { Location } from 'history';
import { BrowserRouter, useLocation, useNavigate, Location as RouterLocation } from 'react-router-dom';

const RouteAdapter: React.FunctionComponent<{
  children: React.FunctionComponent<{
    history: {
      replace(location: Location): void;
      push(location: Location): void;
    },
    location: RouterLocation }>;
}> = ({ children }) => {
  const navigate = useNavigate();
  const routerLocation = useLocation();

  const adaptedHistory = React.useMemo(
    () => ({
      replace(location: Location) {
        navigate(location, { replace: true, state: location.state });
      },
      push(location: Location) {
        navigate(location, { replace: false, state: location.state });
      },
    }), [navigate],
  );
  if (!children) {
    return null;
  }
  return children({ history: adaptedHistory, location: routerLocation });
};

and then use the following router:

<BrowserRouter>
     <QueryParamProvider ReactRouterRoute={RouteAdapter as unknown as React.FunctionComponent}>
         {children}
     </QueryParamProvider>
</BrowserRouter>

@optimistic-updt
@jtheberge

@collinalexbell
Copy link

collinalexbell commented Jan 25, 2022

@jtheberge & @optimistic-updt your code is broken. The final return props needs to have location in it. Right now it has routerLocation in it.

@optimistic-updt
Copy link

@kuberlog When I had the location instead of routerLocation, I wasn't able to navigate if I remember correctly.
In the end, I peddled back on upgrading to react-router v6 as I wasn't I couldn't get useQueryParams to work properly.
I might give it another shot sometime

@collinalexbell
Copy link

collinalexbell commented Jan 27, 2022

I finally got it working, but It would reload the children of the provider (basically the whole app) every time I navigated somewhere.

I ended up removing use-query-params altogether and writing my own adapter that kept the useQueryParams API but used react-router's useSearchParams under the hood. Replace a few imports to use my adapter and volia, everything worked without much code change.

@amcdnl
Copy link

amcdnl commented Feb 14, 2022

Has anyone forked/published/etc these fixes? Seems like a lot of potential solutions here but none are complete.

@jrnail23
Copy link

@kuberlog , care to share your solution?

@amcdnl
Copy link

amcdnl commented May 23, 2022

@pbeshai - Any updates on this?

@jrnail23
Copy link

@kuberlog , when you say "but It would reload the children of the provider (basically the whole app) every time I navigated somewhere", do you mean it would re-render the children, or that it would completely unmount and remount them (or what)?

@Shaddix
Copy link
Contributor

Shaddix commented Jul 24, 2022

Here's my 'fork': https://github.com/mcctomsk/react-router-url-params
It has the same API as useQueryParams/useQueryParam (and even uses the same serialize-query-params).

Internally the 'fork' relies on react-router's useSearchParams, so wrapping into <QueryParamsProvider></QueryParamsProvider> isn't needed.

@amcdnl
Copy link

amcdnl commented Jul 25, 2022

@Shaddix - Did you publish it to NPM?

@Shaddix
Copy link
Contributor

Shaddix commented Jul 25, 2022

yep, https://www.npmjs.org/package/react-router-url-params

@pbeshai
Copy link
Owner

pbeshai commented Jul 25, 2022

Hi there, support for React Router 6 has been added in v2.0.0-rc.0. See the changelog for details on migration. Let me know if you have any issues, will likely switch to the full v2 release by end of week.

@pbeshai pbeshai closed this as completed Jul 25, 2022
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

10 participants