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

Thoughts on a createElectronRouter API to support v6.4 data APIs #7

Closed
offirgolan opened this issue Nov 11, 2022 · 13 comments · Fixed by #20
Closed

Thoughts on a createElectronRouter API to support v6.4 data APIs #7

offirgolan opened this issue Nov 11, 2022 · 13 comments · Fixed by #20
Assignees

Comments

@offirgolan
Copy link
Contributor

👋 Hi!

v6.4 of react router introduced data APIs that can only be used via the createBrowserRouter, createHashRouter, and createMemoryRouter.

Thoughts on having this library support a similar wrapper around createHashRouter?

@offirgolan
Copy link
Contributor Author

Following up on this. This is what I'm currently doing:

import type { FC, ReactNode } from 'react';
import { useMemo } from 'react';
import {
  RouterProvider,
  createHashRouter,
  createRoutesFromChildren,
} from 'react-router-dom';

type Routes = Record<string, ReactNode>;

type AppRouterProps = {
  routes: Routes;
};

function getRouteForCurrentWindow(routes: Routes) {
  const selectAllSlashes = /\//g;
  const windowId = window.location.hash
    .split(selectAllSlashes)?.[1]
    ?.toLowerCase();
  const found = Object.keys(routes).find(
    (key) => key.toLowerCase() === windowId
  );

  return found
    ? {
        basename: `/${windowId}`,
        element: routes[found],
      }
    : null;
}

const AppRouter: FC<AppRouterProps> = ({ routes }) => {
  const router = useMemo(() => {
    const route = getRouteForCurrentWindow(routes);

    return route
      ? createHashRouter(createRoutesFromChildren(route.element), {
          basename: route.basename,
        })
      : null;
  }, [routes]);

  return router ? <RouterProvider router={router} /> : null;
};

export { AppRouter, type AppRouterProps };

@daltonmenezes
Copy link
Owner

Hi @offirgolan I think I don't quite understand which APIs you are referring to. Could you please talk more about it and include links, so I can take a look? Currently, the way it is, what's the issue using both libs?

@offirgolan
Copy link
Contributor Author

@daltonmenezes this is documented here

Screen Shot 2022-11-21 at 9 57 53 AM

@daltonmenezes
Copy link
Owner

Thanks, @offirgolan . Have you had the following type issues?
Captura de tela de 2022-11-24 21-12-27

@daltonmenezes
Copy link
Owner

@offirgolan Ok, the type issue was in v6.4.0, bumping to v6.4.3 solves. 🤔

@daltonmenezes
Copy link
Owner

@offirgolan using react-router-dom v6.6.0, but we need a electron-router-dom breaking-change to make it work only from that and higher versions. 🤔

output.mp4

@offirgolan
Copy link
Contributor Author

@daltonmenezes thanks for taking the time to look into this 😄

I think a new major version release for this breaking change makes sense since it will be following react-router-dom's new API recommendations.

@galetahub
Copy link

@daltonmenezes thanks for your work!
Do you have any plans to support react-router-dom 6.x?

@daltonmenezes
Copy link
Owner

@galetahub soon, I'm finishing the new version of opendocs, after that I can return to electron-router-dom development

@daltonmenezes
Copy link
Owner

daltonmenezes commented Aug 17, 2024

@galetahub @offirgolan

The initial decisions I made for v2.0:

  • simplify the api, electron-router-dom will only export the Router from react-router-dom, the Route and anything else from react-router-dom will need to be imported from it and no longer from electron-router-dom. I believe this will bring less confusion!
  • I added the optional _providerProps property, so that you can override the internal RouterProvider props
  • minimum versions required:
    • react: >=18.0
    • react-router-dom >=6.22.3

what do you think?

code

@daltonmenezes daltonmenezes self-assigned this Aug 17, 2024
@daltonmenezes
Copy link
Owner

daltonmenezes commented Aug 24, 2024

@galetahub @offirgolan how about this one?

  • typed route ids
  • id values inference on createRoute function and Router component
  • simplification of route creation on main process
electron-router.mp4

@daltonmenezes
Copy link
Owner

daltonmenezes commented Sep 8, 2024

@galetahub @offirgolan v2 release planned for this week! Currently working on site, docs and migration guide!
Captura de Tela 2024-09-08 às 18 11 58

@daltonmenezes daltonmenezes linked a pull request Sep 12, 2024 that will close this issue
@daltonmenezes
Copy link
Owner

@galetahub @offirgolan v2 released now! 🎉

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

Successfully merging a pull request may close this issue.

3 participants