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

Automate redirects #621

Closed

Conversation

joysamaddar
Copy link

@joysamaddar joysamaddar commented Dec 14, 2024

An alternate solution that should work with static sites that uses a post-build script to generate the redirection html pages.

P.S. It's was a really rough and quick implementation for a POC. Haven't polished up the code or the html file generated. The generated html files also needs a dexter themed design.

@joysamaddar joysamaddar requested a review from a team as a code owner December 14, 2024 02:45
@dcts
Copy link
Contributor

dcts commented Dec 14, 2024

This seems to pass tests, but will need to check if works in production still. But as you shared in DMs, it might create issues due to collisions (if a redirect route is being created inside the app again), leading to potentially inconsistencies.

Not sure if maybe such an approach would work (this works for a SPA I believe, but we have next export, so each page has a static prerendered HTML, not sure if applicable, just leaving here in case you have an idea):

const redirectRules: Record<string, string> = {
  '/route': 'target',
  '/route2': 'target2',
};

// Function to handle redirects
export function handleRedirects(): boolean {
  const redirectUrl = redirectRules[window.location.pathname];
  if (redirectUrl) {
    window.location.replace(redirectUrl);
    return true; // Redirect occurred
  }
  return false; // No redirect occurred
}

// Handle redirects early
if (!handleRedirects()) {
  // Only initialize React if the target is not a redirect
  createRoot(document.getElementById("root")!).render(
    <>
      {/* <StrictMode> */}
        <StoreProvider>
          <RouterProvider router={router} />
        </StoreProvider>
      {/* </StrictMode> */}
    </>
  );
}

@dcts
Copy link
Contributor

dcts commented Dec 14, 2024

Or maybe an even easier approach would be to make a component, and then instead of creating a page for each page, simply use that component?

Not sure if possible again, just throwing some ideas.

@joysamaddar
Copy link
Author

joysamaddar commented Dec 15, 2024

This seems to pass tests, but will need to check if works in production still. But as you shared in DMs, it might create issues due to collisions (if a redirect route is being created inside the app again), leading to potentially inconsistencies.

The issue due to collisions wouldn't occur with this approach as i don't replace the files in those conditions. Yes, someone could potentially be unaware of a redirect and create a route for that path tho which then takes precedence. A solution to mitigate this could be to possibly throw an error in the build step in those scenarios and stop the build with appropriate error message prompting them to either remove the redirect or remove the page.

@joysamaddar
Copy link
Author

Not sure if maybe such an approach would work (this works for a SPA I believe, but we have next export, so each page has a static prerendered HTML, not sure if applicable, just leaving here in case you have an idea):

const redirectRules: Record<string, string> = {
  '/route': 'target',
  '/route2': 'target2',
};

// Function to handle redirects
export function handleRedirects(): boolean {
  const redirectUrl = redirectRules[window.location.pathname];
  if (redirectUrl) {
    window.location.replace(redirectUrl);
    return true; // Redirect occurred
  }
  return false; // No redirect occurred
}

// Handle redirects early
if (!handleRedirects()) {
  // Only initialize React if the target is not a redirect
  createRoot(document.getElementById("root")!).render(
    <>
      {/* <StrictMode> */}
        <StoreProvider>
          <RouterProvider router={router} />
        </StoreProvider>
      {/* </StrictMode> */}
    </>
  );
}

Isn't

createRoot(document.getElementById("root")!)

for plain react and is abstracted away in nextjs? Haven't used nextjs for a while.

@joysamaddar
Copy link
Author

joysamaddar commented Dec 15, 2024

Or maybe an even easier approach would be to make a component, and then instead of creating a page for each page, simply use that component?

Not sure if possible again, just throwing some ideas.

I did do something similar, but then the question arises on where to wrap the component or add it in.
We could make a wildcard catch all route - [route].ts on the root level which would catch all routes other than the pre-defined routes and then manually redirect or throw the user to the 404 page if a route didn't exists. But imo that's a way worse solution.

I did also put it in the 404 page, since that would catch all non-folder defined routes and then parse the redirection urls and redirect them but we then had the flashing issue of the 404 page.

Also tried adding a "beforeInteractive" inline script on the head of the nextjs app and then redirecting the user if the pathname was a part of a redirect route. Which also caused flashing of the 404 page (the redirect always occurred after the page was fully loaded)

Also, i read that a few hosting providers do provide configs to add in redirects in their config files but I guess we would want to be independent on which hosting provider to use. I see cloudflare pages being used for preview branches and github pages for the prod build?

@dcts
Copy link
Contributor

dcts commented Dec 15, 2024

for plain react and is abstracted away in nextjs? Haven't used nextjs for a while.

You're absolutely right—this was just a suggestion for the logic, not the exact syntax.

I did also put it in the 404 page, since that would catch all non-folder defined routes and then parse the redirection urls and redirect them but we then had the flashing issue of the 404 page.

Your approach sounds excellent! Let’s go with it! 🔥
To fix the 404 flashing issue, you can show an alternative redirect UI while the redirection is in progress. It's worth noting that avoiding temporary screens entirely would require switching to server components. In the current version, this behavior is expected (e.g., if you visit dexteronradix.com/roadmap and simulate "slow 3G" in dev tools, you'll see a blank page before the redirect processes).

You can use usePathname and useRouter to handle the current path and redirects. For example:

export default function NotFound() {
  const pathname = usePathname();
  const router = useRouter();

  // Redirect logic
  if (isRedirectPath(pathname)) {
    router.push(redirectUrl);
    return <p className="text-sm p-6">Redirecting...</p>;
  }

  // Regular 404 page
  return (...);
}

This code is untested, so you might encounter some issues. You can store the redirects in a separate file src/app/redirects.ts for better organization.

@joysamaddar
Copy link
Author

for plain react and is abstracted away in nextjs? Haven't used nextjs for a while.

You're absolutely right—this was just a suggestion for the logic, not the exact syntax.

I did also put it in the 404 page, since that would catch all non-folder defined routes and then parse the redirection urls and redirect them but we then had the flashing issue of the 404 page.

Your approach sounds excellent! Let’s go with it! 🔥

To fix the 404 flashing issue, you can show an alternative redirect UI while the redirection is in progress. It's worth noting that avoiding temporary screens entirely would require switching to server components. In the current version, this behavior is expected (e.g., if you visit dexteronradix.com/roadmap and simulate "slow 3G" in dev tools, you'll see a blank page before the redirect processes).

You can use usePathname and useRouter to handle the current path and redirects. For example:

export default function NotFound() {

  const pathname = usePathname();

  const router = useRouter();



  // Redirect logic

  if (isRedirectPath(pathname)) {

    router.push(redirectUrl);

    return <p className="text-sm p-6">Redirecting...</p>;

  }



  // Regular 404 page

  return (...);

}

This code is untested, so you might encounter some issues. You can store the redirects in a separate file src/app/redirects.ts for better organization.

The issue with this would be that we don't have the pathname readily available and on initial page render its undefined for a moment. That would cause the 404 screen to flash momentarily either way. If we do go ahead with this way, we have 2 options -

  1. User sees a flash of 404 page for a brief second.
  2. We could maybe show a blank page until we have the pathname available. But then for actual 404 page, we see a blank page for a moment.

@dcts
Copy link
Contributor

dcts commented Dec 16, 2024

usePathname is synchronous and in most cases should be available, but yes reading documentation it is actually state that it can return null when hydration happens. But we could still catch those by having another if clause (if pathname === null) then it shows "Loading...".

That could lead to the 404 page showing a loading state briefly, which isn't the cleanest solution, but would not concern me. The benefit of having redirects centralized would make it worth having this "imperfection" IMO.

So the logic would be:

  • if pathname === null -> show "loading..."
  • if pathname is a redirectPath -> show "redirecting..."
  • else -> show regular 404 page

@dcts
Copy link
Contributor

dcts commented Jan 10, 2025

Already implemented in #627

@dcts dcts closed this Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants