pbeshai / use-query-params

React Hook for managing state in URL query parameters with easy serialization.
https://pbeshai.github.io/use-query-params
ISC License
2.15k stars 96 forks source link

React-router v6 TypeScript issues #196

Closed jtheberge closed 2 years ago

jtheberge commented 2 years ago

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 commented 2 years ago

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: https://github.com/remix-run/history/issues/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 commented 2 years ago

Until this lib gets react router v6 support (maybe a complete re-write is better) I created this hook https://github.com/pbeshai/use-query-params/issues/108#issuecomment-977834278 which can be used as useQueryParam. I guess building one that mimics useQueryParams could be done in a similar manner.

pbeshai commented 2 years ago

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 commented 2 years ago

Hey @pbeshai! I think we need to cast it because of https://github.com/pbeshai/use-query-params/blob/452aecddd960ef311a16fd7621fd968c43e0bd83/packages/use-query-params/src/QueryParamProvider.tsx#L134. It seems to need to be a React functional or class component.

ryan-williams commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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

jrnail23 commented 2 years ago

@kuberlog , care to share your solution?

amcdnl commented 2 years ago

@pbeshai - Any updates on this?

jrnail23 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

@Shaddix - Did you publish it to NPM?

Shaddix commented 2 years ago

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

pbeshai commented 2 years ago

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.