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

[v6] Discussion: Impact on SSR after removal of <Redirect /> #7267

Closed
pseudo-su opened this issue Apr 19, 2020 · 23 comments
Closed

[v6] Discussion: Impact on SSR after removal of <Redirect /> #7267

pseudo-su opened this issue Apr 19, 2020 · 23 comments
Labels

Comments

@pseudo-su
Copy link

pseudo-su commented Apr 19, 2020

I'm trying to reconcile how I've been using the <Redirect /> component and figure out how the removal of <Redirect /> is meant to be translated into my "universal" react applications.

tl;dr the removal of <Redirect /> will break the mechanism I've been using to have a consistent redirect experience across server-rendering and client-rendering contexts. I'd like to put my case forward as to why the justification for removing <Redirect /> baffles me. Particularly the suggestion that "you should be doing it on the server anyway", because server-side redirects are precisely one of my use-cases for <Redirect />.

The way I have been using <Redirect /> in the past on the server looks something like this (This won't work as-is but I'm just trying to articulate the gist of the important bits)

// universal/app.js
function App() {
  return (
    <Switch>
      <Route exact path="/" render={() => <Redirect to="/thing1" />} />
      <Route path="/thing1" component={Thing1} />
      <Route path="/thing2" component={Thing2} />
      <Route component={Error404} />
    </Switch>
  )
}

// client/index.js
const mountElement = document.getElementById('root')
hydrate(
  <BrowserRouter>
    <App />
  </BrowserRouter>
  mountElement
)

// server/index.js
function handler(request, response) {
  const reactRouterContext = {};
  const renderedPage = render(
    <StaticRouter location={request.originalUrl} context={reactRouterContext}>
      <App />
    </StaticRouter>
  )

  // Check if the router context contains a redirect. if it does, we need to set
  // the specific status and redirect header and end the response.
  if (reactRouterContext.url) {
    response.status = 302;
    response.set('Location', reactRouterContext.url);
    return;
  }

  response.status = reactRouterContext.missed ? 404 : 200;
  response.body = wrapPageInHTMLTemplate(renderedPage);
}

The recommendation from change log of the v6 alpha is:

If you really need to redirect on the initial render, you can either a) do it on your server (probably best, so it can be cached at the HTTP level instead of doing it in every user's browser) or b) do it outside of React Router (e.g. using the history API directly).

It sounds like it was removed with the assumption it would only affect people using it for client-side redirects... but I'm specifically using <Redirect /> as a mechanism to have a unified redirect experience regardless of whether it's the first server-side render or subsequent client-side renders.

The wording around needing to "do it on your server" implies to me that server-rendering is viewed to be outside the scope of react-router. Is this signposting a deliberate move away from having SSR capability in react-router going forward?

Ultimately if that's the direction this project is going in I can work around it, but I'm looking for clarification first.

EDIT:
I found where this is currently mentioned in the docs so I'll add it here for completeness sake https://reacttraining.com/react-router/web/guides/server-rendering

@pseudo-su pseudo-su changed the title Discussion: removal of <Redirect /> [v6] Discussion: removal of <Redirect /> Apr 19, 2020
@aappddeevv
Copy link

aappddeevv commented Apr 19, 2020

Why would you need to change that (other than syntax and how the components used)? Instead of Redirect there is a Navigate component. I was looking over issues so I can start using v6 and I came across your question and thought I might want to do what you did.

@pseudo-su
Copy link
Author

pseudo-su commented Apr 19, 2020

Are you asking why I can't use <Navigate /> in the same way that I describe my current usage of <Redirect />?

Apart from it being semantically incorrect there's a comment/warning here stating

<Navigate> must not be used on the initial render in a <StaticRouter>. This is a no-op, but you should modify your code so the is only ever rendered in response to some user interaction or state change.

/~https://github.com/ReactTraining/react-router/blob/v6.0.0-alpha.3/packages/react-router/index.js#L108

@aappddeevv
Copy link

Ah, got it.

@pseudo-su
Copy link
Author

pseudo-su commented May 6, 2020

As of the latest alpha:

Removed the <StaticRouter context> API. We don't support navigation on the initial render in v6, so this API is unnecessary.

I find this confusing because I don't understand the static router context as being related (at least not exclusively) to navigating during the initial render.

EG: application renders a 404 page in a StaticRouter and the SSR wrapper needs to know that it should set the statusCode on the response.

@max-mykhailenko
Copy link

I have the same question and one more about SSR. Previously I was able to get matched route with matchPath, but now as I understand you changed path ideology and I should use another methods. Need your advice

@dimaqq
Copy link

dimaqq commented May 15, 2020

justification for removing ... baffles me

For the less involved, can you link to the original idea to remove?

@pseudo-su
Copy link
Author

@Askadias
Copy link

+1 for that question. If StaticRouter and Redirect removal "due to compatibility issues with future versions of React" is connected with Suspense support, maybe it's worth to move such breaking changes into the @experimental branch?

@pseudo-su
Copy link
Author

pseudo-su commented Jun 12, 2020

Just to be clear @Askadias, I believe that StaticRouter hasn't been removed entirely, but the ability to pass context to it has. Which affects my current usage because i'm using the current canonical way to facilitate rendering different HTTP Status codes with react-router in a universal react app.

FYI: I was somewhat hoping that there might be some clarification with the remix run newsletter:

[...etc]

All hosting services have a way to define redirects for URLs that have changed. Most React apps just set up some client side redirects with React Router and call it a day. The problem is the search engine bot gets a 200, when it should get a 30x! Now your page rank is going to get effected negatively for those URLs that have moved. For some projects, this might mean a huge hit to revenue, signups, etc.

Instead of configuring this client side, you should configure it on your server. That way the user and the search engine bots get a 30x at the old URL and then a 200 at the new one.

However, if you only configure you redirect on the server, then any links in your app that haven't been updated will not get redirected when the user clicks them even though it's a valid page.

Holy smokes ... so now, to do this properly, you have to configure redirects on the server AND the client.

But ... that's still not enough 😅 If a bot follows a redirected link, the page will update client side but the bot will not get the proper status codes!

Perhaps this is why few people tackle it, it's a little wild.

In Remix, you only have to configure your redirects on your server, using whatever API your platform or host provides. Usually there's a JSON file to configure them. If the user lands on a redirected page, the server is the only thing involved and does the right thing. If the user or a bot clicks a link to a redirect page, just like in scenario (3), Remix doesn't match any routes in the client, so it goes back to the server with a new page load, then the server takes over and sends the right status code.

I'm still primarily confused about why there's an assumption that it needs to be "configured once on the client and once on the server". My use case for <Redirect /> (and the other described usages of static context) is precisely so that I only have to define the behavior once in my components and get consistent behavior across both server and client.

@Askadias
Copy link

Askadias commented Jun 12, 2020

My use-case is pretty much similar. We apply a lot of various async logic based on query string in our OAuth application using redux. As result we get a redirect url and render it in router using <Redirect to={redirectUrl}/>. If we apply the same async precondition on server we get the final redux state and as a result retrieve the redirection url from the router context.
We have like ~60 pages, half of them have some async preconditions most of which leads to redirection.

@max-degterev
Copy link

What is the latest status of this? Does someone know if RR6 will support SSR?

@capaj
Copy link

capaj commented Jun 25, 2020

@suprMax I don't know, but removing SSR support from RR6 would be a major issue for a lot of people. I don't think anybody wants that.

@max-degterev
Copy link

I'm cautiously optimistic that it won't be removed, I haven't tried RR6 myself yet. I think the amount of effort we've put at our company in keeping up with RR updates pretty much equals what would take to roll our own solution, so this update we will probably drop RR altogether if it's anything like 3->4. I wanted a bit of a heads up what to plan for.

@Zertz
Copy link

Zertz commented Aug 17, 2020

From the example in #7535, I ended up with something similar to this using 6.0.0-beta.0:

<Routes>
  <Route path="a" element={<A />} />
  <Route path="b" element={<B />} />
  <Route element={<Navigate to="a" />} />
</Routes>

It seems to work correctly and is almost equivalent to <Redirect />

@pseudo-su
Copy link
Author

pseudo-su commented Aug 18, 2020

@Zertz <Navigate /> is a perfect adequate replacement for <Redirect /> when rendering on the client.

The problem is that <Navigate /> is a no-op on the server and therefore doesn't cater to the SSR use cases that the current <Redirect /> component does.

Docs here for more details on how <Redirect /> works currently on the server

@pseudo-su pseudo-su changed the title [v6] Discussion: removal of <Redirect /> [v6] Discussion: Impact of removing <Redirect /> for SSR Aug 18, 2020
@pseudo-su pseudo-su changed the title [v6] Discussion: Impact of removing <Redirect /> for SSR [v6] Discussion: Impact of removing <Redirect /> on SSR Aug 18, 2020
@pseudo-su pseudo-su changed the title [v6] Discussion: Impact of removing <Redirect /> on SSR [v6] Discussion: Impact on SSR after removal of <Redirect /> Aug 18, 2020
@stale
Copy link

stale bot commented Oct 17, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
You can add the fresh label to prevent me from taking any action.

@stale stale bot added the stale label Oct 17, 2020
@rstrand
Copy link

rstrand commented Oct 21, 2020

@pseudo-su would it be an option to create your own <Redirect /> component which calls <Navigate /> on the client side and implements your own redirect status code rendering on the server?

@stale stale bot removed the stale label Oct 21, 2020
@pseudo-su
Copy link
Author

pseudo-su commented Oct 22, 2020

@pseudo-su would it be an option to create your own <Redirect /> component which calls <Navigate /> on the client side and implements your own redirect status code rendering on the server?

Yes, it's possible. The reason I opened this issue is because I wasn't sure if this is/was a deliberate deprecation of SSR capabilities or not. Personally I think if the plan is to deprecate/remove the current documented way of doing SSR with react router it'd be a really good idea to at least update the current docs to clarify that things to do with SSR are going to be removed from react-router going forward.

That's the main reason I asked the question:

Is this signposting a deliberate move away from having SSR capability in react-router going forward?

For what it's worth, I'm assuming the silence means that the change will be made and server rendering will be removed from react-router.

It's not a massive inconvenience, but the lack of clarity makes it confusing as to whether people should be doing anything in preperation (if they use staticContext as per current docs).

In case anyone's particularly curious, this is what I did to prepare for the removal of <Redirect /> and staticContext.

/~https://github.com/bootleg-rust/sites/tree/9028da0c40d1261a407479c61dd8c5c27ae47453/packages/lib-ssr-toolbox/src/http

tl;dr basically I have a seperate HttpProvider and have copied/duplicated some of the current react-router implementation for things like <Redirect /> etc into my own codebase and made them use my own httpContext instead of react-router's staticContext. I know I'll have to update them when moving to v6 (and potentially also figure out potential issues w/ suspsense triggering side-effects for "uncommited" renders).

// universal/app.js
function Dashboard() {
  return <>
    Home page
    <CacheControl maxAge={200} />
  </>
}

function Error404() {
  return <>
    <HttpStatus code={400} />
    {/* etc */}
  </>
}

function App() {
  return (
    <Switch>
      <Route exact path="/" render={() => <Redirect to="/dashboard" />} />
      <Route path="/dashboard" component={Dashboard} />
      <Route path="/thing2" component={Thing2} />
      <Route component={Error404} />
    </Switch>
  )
}

// client/index.js
const mountElement = document.getElementById('root')
hydrate(
  <BrowserRouter>
    <App />
  </BrowserRouter>
  mountElement
)

// server/index.js
function handler(ctx: koa.Context) {
  const httpContext: HttpContextData = {
      cacheControl: [],
      statusCode: 200,
    };
  const renderedPage = render(
    <HttpProvider context={httpContext}>
      <StaticRouter location={ctx.request.url}>
        <App />
      </StaticRouter>
    </HttpProvider>
  )

  // Redirect when <Redirect /> is rendered
  if (httpContext.redirectLocation) {
    // Somewhere a `<Redirect>` was rendered
    ctx.redirect(httpContext.redirectLocation);
    return;
  }

  // Handle status codes
  ctx.status = 200;
  if ([400, 401, 402, 403, 404].includes(httpContext.statusCode)) {
    ctx.status = httpContext.statusCode;
  }
  // Set cache-control based on rendered content
  const cacheControl = reconcileCacheControlOptions(httpContext);
  const cacheControlValue = format(cacheControl);
  if (cacheControlValue) {
    ctx.set("Cache-Control", cacheControlValue);
  }

  ctx.response.body = wrapPageInHTMLTemplate(renderedPage);
}

@max-degterev
Copy link

max-degterev commented Oct 22, 2020

We (at work) will implement a RR alternative with full SSR support and API similar to RR5. I will link here once we have a stable implementation in production.

Does anyone know when RR6 is going to be released? We will try to time it the same.

@stale
Copy link

stale bot commented Dec 22, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
You can add the fresh label to prevent me from taking any action.

@stale stale bot added the stale label Dec 22, 2020
@stale stale bot closed this as completed Dec 29, 2020
@t-pyrope
Copy link

t-pyrope commented Nov 5, 2021

For me worked to pass "*" as a path of the last route (path="*"), without that it didn't work

<Routes>
    <Route path="home" element={<HomePage />} />
    <Route path="*" element={<Navigate to="home" />} />
</Routes>

@calvinaco
Copy link

calvinaco commented Nov 17, 2021

If you have defined the parent Route with a layout, you can try the following code snippet

<Routes>
    <Route path="/" element={<Layout />}>
        <Route path="a" element={<A />} />
        <Route path="b" element={<B />} />
        <Route index element={<Navigate to="a" />} />
    </Routes>
</Routes>

@t-fritsch
Copy link

Sorry to dig-up this old issue, but it has been automatically closed 3 years ago, and as far as I understand the discussion there is still no built-in way -without a "Data Router"- to detect redirection server side as it was possible with the context before, shouldn't this issue be reopened and worked on ? Did I miss some solution to the problem ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests