atellmer / dark

The lightweight and powerful UI rendering engine without dependencies and written in TypeScript💫 (Browser, Node.js, Android, iOS, Windows, Linux, macOS)
MIT License
40 stars 1 forks source link

Redirect/navigate on wildcard, how? #53

Closed Coachonko closed 2 months ago

Coachonko commented 2 months ago

I have an internationalized application, paths are prefixed with the current language. I would like to redirect the visitor using the data contained in the path, as you can see in this component here. How would I do that? I think the information that most closely describes the Dark web-router API, that I can find, is this one here, is this how?

atellmer commented 2 months ago

You need to use the useHistory hook

const TranslatedNotFound = component(() => {
  const history = useHistory();

  useLayoutEffect(() => {
    if (notFound) {
      history.push('/new/route');
      // if don't need to save the current route in the browser history
      // history.replace('/new/route');
    }
  }, [somedep]);

  return null;
});

It seems I forgot to write about it in the readme.

Coachonko commented 2 months ago

Thank you, this is good information.

Also I think I'm stupid. I should just wildcard ${lang}/**

{
  path: "/**",
  component: [Function],
  translated: true,
  language: "en",
}, {
  path: "/it/**",
  component: [Function],
  translated: true,
  language: "it",
}
]
// attempting to navigate to /it/lmao

110 | }
111 | function canRender() {
112 |   return route => {
113 |     if (route?.component) return route;
114 |     if (process.env.NODE_ENV !== 'test') {
115 |       throw new Error('[web-router]: the route not found or it has no component!'); // missing `was`
                ^
error: [web-router]: the route not found or it has no component!
    at /home/jdb/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/web-router/dist/esm/create-routes/create-routes.js:115:13
    at resolveRoute (/home/jdb/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/web-router/dist/esm/create-routes/create-routes.js:195:23)
    at /home/jdb/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/web-router/dist/esm/router/router.js:29:52
    at mount (/home/jdb/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/core/dist/esm/workloop/workloop.js:291:20)
    at share (/home/jdb/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/core/dist/esm/workloop/workloop.js:156:18)
    at mountChild (/home/jdb/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/core/dist/esm/workloop/workloop.js:122:3)
    at performUnitOfWork (/home/jdb/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/core/dist/esm/workloop/workloop.js:96:19)
    at workLoop (/home/jdb/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/core/dist/esm/workloop/workloop.js:66:14)
    at performWorkUntilDeadline (/home/jdb/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/core/dist/esm/scheduler/scheduler.js:153:27)
    at forEach (:1:21)

Maybe this is fixed with the new changes to web-router.

Can you npm publish a new version of Dark with the new changes?

atellmer commented 2 months ago

In your case with lang you can use this approach - lang as parameter

import { type DarkElement, h, Fragment, component, useEffect } from '@dark-engine/core';
import { createRoot } from '@dark-engine/platform-browser';
import { type Routes, Router, RouterLink, useHistory, useParams } from '@dark-engine/web-router';

const Shell = component<{ slot: DarkElement }>(({ slot }) => {
  const params = useParams();
  const history = useHistory();
  const lang = params.get('lang');

  useEffect(() => {
    if (lang !== 'en' && lang !== 'it') {
      history.replace('/en/home');
    }
  }, [lang]);

  return (
    <div>
      Shell {lang} {slot}
    </div>
  );
});

const Nav = component(() => {
  const params = useParams();
  const lang = params.get('lang');

  return (
    <nav>
      <RouterLink to={`/${lang}/home`}>Home</RouterLink>
      <RouterLink to={`/${lang}/contacts`}>Contacts</RouterLink>
      <RouterLink to={`/${lang}/about`}>About</RouterLink>
      <RouterLink to='/xxx'>404</RouterLink>
    </nav>
  );
});

const Home = component(() => {
  return <div>home</div>;
});

const Contacts = component(() => {
  return <div>Contacts</div>;
});

const About = component(() => {
  return <div>About</div>;
});

const routes: Routes = [
  {
    path: ':lang',
    component: Shell,
    children: [
      {
        path: 'home',
        component: Home,
      },
      {
        path: 'about',
        component: About,
      },
      {
        path: 'contacts',
        component: Contacts,
      },
      {
        path: '',
        redirectTo: 'home',
      },
      {
        path: '**',
        redirectTo: 'home',
      },
    ],
  },
  {
    path: '',
    redirectTo: ':lang/home',
  },
  {
    path: '**',
    redirectTo: ':lang/home',
  },
];

const App = component(() => {
  return (
    <Router routes={routes}>
      {slot => (
        <main>
          <Nav />
          {slot}
        </main>
      )}
    </Router>
  );
});

createRoot(document.querySelector('#root')).render(<App />);

Wilcard always will be just **, not some/**

I will publish the packages after small minor changes soon. (possibly tomorrow)

Coachonko commented 2 months ago

In your case with lang you can use this approach - lang as parameter

I really don't remember why but I purposely avoided that with react router and inferno router before. I remember running into a messy problem. Since then I just iterate and register an independent route for each language instead. I may be crazy but I think it may be less computationally expensive to have a large routes array instead of a param. I will try this approach and see if I run into issues and remember what the issue was.

Off the top of my head I can see a problem already: the default language has no language prefix in the path in my current implementation, only secondary languages have the prefix

Wilcard always will be just **, not some/**

Oh. Alright. Then useHistory is the solution for me

atellmer commented 2 months ago

I will try this approach and see if I run into issues and remember what the issue was.

Ok. it will be intresting I think.

Coachonko commented 2 months ago

I am attempting what you suggested

const baseRoutes = [
  {
    path: '/',
    pathMatch: 'full',
    component: lazy(() => import('../pages/Home'))
  },
  {
    path: 'contact',
    pathMatch: 'full',
    component: lazy(() => import('../pages/Contact'))
  },
  {
    path: 'not-found',
    pathMatch: 'full',
    component: lazy(() => import('../pages/NotFound'))
  },
  {
    path: '**',
    redirect: 'not-found'
  }
]

export const routes = [
  {
    path: '',
    component: lazy(() => import('../components/Root')),
    children: [...baseRoutes]
  },
  {
    path: ':lang',
    component: lazy(() => import('../components/Root')),
    children: [...baseRoutes]
  }
]

I am trying this, but I am experiencing something I don't understand. navigating to /de for example, immediately redirects to /. I have no redirects anywhere, except for the wildcard routes.

root rendered
match:  {
  path: "/:lang/",
  url: "/de/",
}
params:  Map {}
home rerendered

It looks like /de matches the :lang route, but the params map is empty. Attempting to visit /de/contact results in an error and no renders.

110 | }
111 | function canRender() {
112 |   return route => {
113 |     if (route?.component) return route;
114 |     if (process.env.NODE_ENV !== 'test') {
115 |       throw new Error('[web-router]: the route not found or it has no component!');
                  ^
error: [web-router]: the route not found or it has no component!
      at /home/coachonko/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/web-router/dist/esm/create-routes/create-routes.js:115:13
      at resolveRoute (/home/coachonko/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/web-router/dist/esm/create-routes/create-routes.js:195:23)
      at /home/coachonko/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/web-router/dist/esm/router/router.js:29:52
      at mount (/home/coachonko/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/core/dist/esm/workloop/workloop.js:291:20)
      at share (/home/coachonko/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/core/dist/esm/workloop/workloop.js:156:18)
      at mountChild (/home/coachonko/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/core/dist/esm/workloop/workloop.js:122:3)
      at performUnitOfWork (/home/coachonko/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/core/dist/esm/workloop/workloop.js:96:19)
      at workLoop (/home/coachonko/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/core/dist/esm/workloop/workloop.js:66:14)
      at performWorkUntilDeadline (/home/coachonko/Documents/projects/frontend/dark-coachonko.com/node_modules/@dark-engine/core/dist/esm/scheduler/scheduler.js:153:27)
      at forEach (:1:21)

I have a branch here

atellmer commented 2 months ago

Try this code (very similar), it works fine.

import { type DarkElement, component, useEffect } from '@dark-engine/core';
import { createRoot } from '@dark-engine/platform-browser';
import { type Routes, Router, RouterLink, useHistory, useParams } from '@dark-engine/web-router';

const Root = component<{ slot: DarkElement }>(({ slot }) => {
  const params = useParams();
  const history = useHistory();
  const lang = params.get('lang');

  useEffect(() => {
    if (lang !== 'en' && lang !== 'it') {
      history.replace('/en');
    }
  }, [lang]);

  return (
    <div>
      Root {lang} {slot}
    </div>
  );
});

const Nav = component(() => {
  const params = useParams();
  const lang = params.get('lang');

  console.log('params', params);

  return (
    <nav>
      <RouterLink to={`/${lang}`}>Home</RouterLink>
      <RouterLink to={`/${lang}/contacts`}>Contacts</RouterLink>
      <RouterLink to={`/${lang}/xxx`}>404</RouterLink>
    </nav>
  );
});

const Home = component(() => {
  return <div>Home</div>;
});

const Contacts = component(() => {
  return <div>Contacts</div>;
});

const NotFound = component(() => {
  return <div>Not Found</div>;
});

const routes: Routes = [
  {
    path: ':lang',
    component: Root,
    children: [
      {
        path: '',
        component: Home,
      },
      {
        path: 'contacts',
        component: Contacts,
      },
      {
        path: 'not-found',
        component: NotFound,
      },
    ],
  },
  {
    path: '**',
    redirectTo: ':lang/not-found',
  },
];

const App = component(() => {
  return (
    <Router routes={routes}>
      {slot => (
        <main>
          <Nav />
          {slot}
        </main>
      )}
    </Router>
  );
});

createRoot(document.querySelector('#root')).render(<App />);

Also you can remove the part with

{
    path: '',
    component: lazy(() => import('../components/Root')),
    children: [...baseRoutes]
},

because '' less specific than :lang and is found first. Even if both worked /contact and /en - both will be perceived as :lang.

Also I published 1.1.0 version of packages.

Coachonko commented 2 months ago

I see, I think I understand, but not fully.

The thing is I want default-language routes to have no language prefix. For example / is default language /de is alternate language (german) /contact is default language /de/contact is alternate language (german) I was hoping that by placing / above, it would match the non-prefixed paths first and fallback to prefixed paths after.

export const routes = [
  {
    path: '',
    children: [
      ...baseRoutes,
      {
        path: ':lang',
        component: lazy(() => import('../components/Root')),
        children: [...baseRoutes]
      }
    ]
  }
]

I tried this too and that caused a lot of errors :laughing:

atellmer commented 2 months ago

Then that changes things. In this case, the parameters will not help. Only a static description of routes will help, as you did earlier.

I tried this too and that caused a lot of errors 😆

haha, I think for this to work, the router must have intelligence like chatGPT.

for case above

const routes: Routes = [
  {
    path: 'contacts',
    component: component(() => Root({ slot: Contacts() })), // bad
  },
  {
    path: 'not-found',
    component: NotFound,
  },
  {
    path: 'de',
    component: Root,
    children: [
      {
        path: 'contacts',
        component: Contacts,
      },
      {
        path: '**',
        pathMatch: 'full',
        redirectTo: '/not-found',
      },
    ],
  },
  {
    path: '',
    component: Root,
  },
  {
    path: '**',
    redirectTo: 'not-found',
  },
];
atellmer commented 2 months ago

We need to support something like this

const routes: Routes = [
  {
    path: 'de',
    component: Root,
    children: [
      {
        path: 'contacts',
        component: Contacts,
      },
      {
        path: '**',
        pathMatch: 'full',
        redirectTo: '/not-found',
      },
    ],
  },
  {
    path: '',
    component: Root,
    children: [
      {
        path: 'contacts',
        component: Contacts,
      },
      {
        path: 'not-found',
        component: NotFound,
      },
    ],
  },
  {
    path: '**',
    redirectTo: 'not-found',
  },
];
Coachonko commented 2 months ago

Now I wonder how other routers handle this. I am only aware of nextjs offering this by default, but they use a very different routing strategy so I doubt it can be applied to Dark. I believe there is a third party package for react router 6. I don't know how angular does i18n routes, but I am sure there is a technique to handle them nicely.

If you decide to support some sort of i18n routing on web-router I think it should be able to handle any number of alternate languages, not just one.

I have upgraded to 1.1.0 and fixed up the routing in my repository. I think I made it work fine by generating a large array, one route object for each route and no children.

atellmer commented 2 months ago

I think it should be able to handle any number of alternate languages, not just one.

Yea, I mean

const routes: Routes = [
  ...['en', 'it', 'fr'].map(lang => ({
    path: lang,
    component: Root,
    children: [
      {
        path: 'contacts',
        component: Contacts,
      },
      {
        path: '**',
        pathMatch: 'full',
        redirectTo: '/not-found',
      },
    ],
  })),
  {
    path: '',
    component: Root,
    children: [
      {
        path: 'contacts',
        component: Contacts,
      },
      {
        path: 'not-found',
        component: NotFound,
      },
    ],
  },
  {
    path: '**',
    redirectTo: 'not-found',
  },
];

Hope it won't be difficult.

Coachonko commented 2 months ago

I was investigating how other routers work in other frameworks, the following works in React Router

const routes = [
  {
    path: '/',
    element: <Root />,
    children: [
      {
        path: '', // matches /
        element: <Home />
      },
      {
        path: 'contact', //matches /contact
        element: <Contact />
      },
      {
        path: 'de',
        children: [
          {
            path: '', // matches /de
            element: <Home />
          },
          {
            path: 'contact', // matches /de/contact
            element: <Contact />
          }
        ]
      }
    ]
  }
]

Just noting it here.

atellmer commented 2 months ago

Thanks for your investigation. With some fixes in open PR it will work pretty much the same way.

const routes = [
  {
    path: '/',
    component: Root,
    children: [
      {
        path: '/', // matches /
        component: Home,
      },
      {
        path: 'contact', //matches /contact
        component: Contact,
      },
      {
        path: 'de',
        component: component(({ slot }) => slot), // should be here
        children: [
          {
            path: '/', // matches /de
            component: Home,
          },
          {
            path: 'contact', // matches /de/contact
            component: Contact,
          },
        ],
      },
    ],
  },
  {
    path: '**',
    redirectTo: '/',
  },
];
Coachonko commented 2 months ago

Awesome, the fix you pushed works great. Thank you, this greatly simplifies my app.

I have a question:

export const baseRoutes = [
  {
    path: '/',
    component: lazy(() => import('../pages/Home')),
    seo: 'home'
  },
  {
    path: '/contact',
    component: lazy(() => import('../pages/Contact')),
    seo: 'contact'
  },
  {
    path: '/not-found',
    component: lazy(() => import('../pages/NotFound'))
  }
]

function generateRoutes (baseRoutes) {
  const alternateLanguageRoutes = []
  for (let i = 1, len = languages.length; i < len; i++) { // skip default language
    const language = languages[i]
    alternateLanguageRoutes.push({
      path: `/${language}`,
      component: Fragment,
      children: [
        ...baseRoutes,
        {
          path: '**',
          redirectTo: '/not-found'
        }
      ]
    })
  }
  return [
    {
      path: '/',
      component: Root,
      children: [
        ...baseRoutes,
        ...alternateLanguageRoutes,
        {
          path: '**',
          redirectTo: '/not-found'
        }
      ]
    }
  ]
}

export const routes = generateRoutes(baseRoutes)

The above works, however if I add pathMatch: full to /contact, the app redirects /de/contact to /de. And if I add pathMatch: full to /not-found, then the app breaks when requesting /de/contact. Any clue why?

The JSON.strigify(routes) output is as follows.

[
  {
    "path": "/",
    "children": [
      {
        "path": "/",
        "seo": "home"
      },
      {
        "path": "/contact",
        "pathMatch": "full",
        "seo": "contact"
      },
      {
        "path": "/not-found",
        "pathMatch": "full"
      },
      {
        "path": "/de",
        "children": [
          {
            "path": "/",
            "seo": "home"
          },
          {
            "path": "/contact",
            "pathMatch": "full",
            "seo": "contact"
          },
          {
            "path": "/not-found",
            "pathMatch": "full"
          },
          {
            "path": "**",
            "redirectTo": "/not-found"
          }
        ]
      },
      {
        "path": "**",
        "redirectTo": "/not-found"
      }
    ]
  }
]

Update: maybe I am wrong but I think RouterLink broke. 1.1.1 must have introduced a regression in the RouterLink component. In some situations it seems to work fine, in other situations it doesn't change route or causes a full-page reload.

atellmer commented 2 months ago

Routing breaks because you use pathMatch: 'full' incorrectly. full means that you must always write the path from start. In most cases it useful if you want make a redirect from child route to any parent or sibling route (into another place of routes tree). In the code above no need using full option.

Your worked example 1

Coachonko commented 2 months ago

Routing breaks because you use pathMatch: 'full' incorrectly. full means that you must always write the path from start. In most cases it useful if you want make a redirect from child route to any parent or sibling route (into another place of routes tree). In the code above no need using full option.

Oh I understand! So the route must be defined with the full path to match the path fully. Thank you.

Regarding RouterLink, please take a look at this component I have written some notes in a comment block regarding what I am experiencing

atellmer commented 2 months ago

I have trouble with your bun server. Any ideas? (Bun installed)

npm start

> start
> bun run ./server/index.js

18 |   }
19 | }
20 |
21 | async function generateSitemapIndex (paths) {
22 |   const indexPath = `${process.cwd()}${paths.index}`
23 |   const file = Bun.file(indexPath)
                    ^
TypeError: Failed to create FileSink: src.windows_c.E.NOENT
 code: "ERR_INVALID_ARG_TYPE"

      at D:\apps\coachonko.com\server\sitemaps.js:23:16
      at generateSitemapIndex (D:\apps\coachonko.com\server\sitemaps.js:21:38)
      at D:\apps\coachonko.com\server\sitemaps.js:14:11
      at generateSitemaps (D:\apps\coachonko.com\server\sitemaps.js:7:40)
      at D:\apps\coachonko.com\server\index.js:7:7
Coachonko commented 2 months ago

Are you on windows? I am on linux so I can't reproduce

atellmer commented 2 months ago

Yes, win 11. Anyway server started. And I see broken html there 1

It seems I will long debug tomorrow

Coachonko commented 2 months ago

Interesting, I did not even see that happening. I can confirm, it happens on my end too.

I think that <br /> is breaking it somehow. I also notice that router-link-active is being applied to both language links sometimes

atellmer commented 2 months ago

The problem in platform-server I think. It breaks on <br /> and returns inconsistent html with <br></br> tag. This is interpreted by the browser as two <br> tag and we have hydration error under the hood.

Yea, we need think about better algorithm for match active links. For example it should create two class router-link-active and router-link-exact. Tomorrow I will try to fix these bugs.

Coachonko commented 2 months ago

Thank you for taking a look at the issues.

I have noticed that the server sends a <head> that contains some duplicate </script> tags too. Probably caused by the same bug

<html>
    <head>
        <meta charset="UTF-8"></meta>
        <meta name="viewport" content="width=device-width, initial-scale=1"></meta>
        <base href="/"></base>
        <title>fallback title</title>
        <meta name="robots" content="noindex, nofollow"></meta>
        <link rel="canonical" href="https://coachonko.com/not-found"></link>
        <!--dark:matter-->
        <link rel="shortcut icon" href="/public/favicon.ico"></link>
        <script type="module" src="/assets/index.js" defer></script>
</script></head>

As you can see in my component, I already am able to detect whether or not the link is active, maybe there should be one RouterLink version that does not add any active class, I can add my own.

Also noticed that the active class is not documented.

atellmer commented 2 months ago

version 1.2.0 is available