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: Optional parameters #7285

Closed
clarkd opened this issue Apr 27, 2020 · 62 comments
Closed

V6: Optional parameters #7285

clarkd opened this issue Apr 27, 2020 · 62 comments

Comments

@clarkd
Copy link

clarkd commented Apr 27, 2020

I tried adding a trailing ? to a route using the v6 router, but it didn't seem to work.

<Route path='/page/:friendlyName/:sort?' element={<Page/>} />

Are optional parameters supported in v6, or are they coming later?

@timdorr
Copy link
Member

timdorr commented Apr 27, 2020

We don't plan on supporting them in v6.

@timdorr timdorr closed this as completed Apr 27, 2020
@clarkd
Copy link
Author

clarkd commented Apr 28, 2020

@timdorr Thanks - could you explain a little on how you would achieve it in v6? At the moment I have the below which works but seems less than optimal. Any thoughts?

<Route path='/page/:friendlyName/:sort' element={<Page/>} />
<Route path='/page/:friendlyName/' element={<Page/>} />

@gopeter
Copy link

gopeter commented May 11, 2020

It would be great if this could be described in the migration guide – or if there was at least a hint. So the recommended way of doing "optional" params really is ...

<Route path='/page/:friendlyName/:sort' element={<Page/>} />
<Route path='/page/:friendlyName/' element={<Page/>} />

... ?

@MeiKatz
Copy link
Contributor

MeiKatz commented May 11, 2020

@gopeter Yes, or you can do something like this:

<Route path="/page/:friendlyName">
  <Route path=":sort" element={<Page />} />
  <Route path="" element={<Page />} />
</Route>

@gopeter
Copy link

gopeter commented May 11, 2020

Aaaaah, great, thank you very much! 😄

@robbinjanssen
Copy link

robbinjanssen commented Sep 15, 2020

@MeiKatz @timdorr my problem with the approach above is that the <Page> component unmounts and mounts again. Performance-wise this might be a bad thing, but in my case it also results in a bunch of refetching of data. I can fix this inside the Page-component itself by checking if data is there already, but not only does this change break my routing, it also breaks application itself.

This is my use case;

<Route path="tickets">
  <Routes>
    <Route path="archive">
      <Route path=":ticketId/:content" element={<List />} />
      <Route path=":ticketId" element={<List />} />
      <Route path="/" element={<List />} />
    </Route>
    <Route path="view/:ticketId">
      <Route path=":content" element={<List />} />
      <Route path="/" element={<List />} />
    </Route>
    <Route path=":ticketId">
      <Route path=":content" element={<List />} />
      <Route path="/" element={<List />} />
    </Route>
    <Route path="/" element={<List />} />
  </Routes>
</Route>

A user starts at /tickets, then clicks on a link on the list that navigates to /tickets/1. This now re-renders the complete page where before it would re-render a part inside the component. Navigating from /tickets/1 to /tickets/2 works as expected.

Any other suggestions on how to implement this (Route-wise) without having to refactor the complete <List/>-component?

@MeiKatz
Copy link
Contributor

MeiKatz commented Sep 15, 2020

@robbinjanssen Have you thought about using the <Outlet /> element?

<Route path="tickets" element={ <TicketsPage /> }>
  <Route path=":ticketId" element={ <ShowPage /> } />
  <Route path="" element={ <IndexPage /> } />
</Route>
const TicketContext = React.createContext( {} );

function TicketsPage() {
  const [ tickets, setTickets ] = useState({});

  useEffect(() => {
    // do some loading stuff
    const res = await fetch("/tickets");
    const tickets = await res.json();

    setTickets( tickets );
  }, []);

  return (
    <Fragment>
      <h1>My Tickets</h1>
      <TicketContext.Provider value={ tickets }>
        <Outlet />
      </TicketContext.Provider>
    </Fragment>
  );
}

function ShowPage() {
  const tickets = useContext( TicketContext );
  const { ticketId } = useParams();

  const ticket = tickets[ ticketId ];

  return (
    <div>{ JSON.stringify( ticket ) }</div>
  );
}

@robbinjanssen
Copy link

robbinjanssen commented Sep 15, 2020

@MeiKatz not yet, hoped I would not have to refactor that ticket page, but seems like there's no other way. It's a pretty "old" component that requires a lot of work when refactoring. So I wish I could somehow keep the optional parameters :)

@MeiKatz
Copy link
Contributor

MeiKatz commented Sep 15, 2020

@robbinjanssen Understandable. But I don't think that there is any other way you can solve it. And I also think, that it is a clean way to do it. Also we could think about dropping the whole context stuff and pass the parameters to the <Outlet /> and from there to the element.

@robbinjanssen
Copy link

For now I'll just parse the "raw" params into the bits I need, those URLs are not that complicated fortunately, and i'll put the component on the refactor list. Thanks :)

@manterfield
Copy link

Is there anywhere public that we can see the reasoning behind the choice to drop this feature? I appreciate the workarounds and help you've provided above @MeiKatz, but they're still a less ergonomic developer experience (IMO at least).

I can't imagine that you'd take that choice without good reason, but it'd be good to know what that was and if it's something that can be overcome?

@marcel-happyfloat
Copy link

Facing a similar issue. I want to open a Modal depending on the optional parameter. The Modal needs to be mounted already in order to show/hide correctly. I don't think using an outlet with immediate mount/unmount will work here. So I'm doing this as a work around. This way I can parse the route params in the HomePage Component, where the Modal is already mounted:

<Route path="/" element={<HomePage />}>
    <Route path=":activity_id" element={null} />
</Route>

@n8sabes
Copy link

n8sabes commented Nov 5, 2021

Dropping v5-style optional parameters is requiring significant code changes to migrate from v5 to v6. Aside from the new v6 optional parameter patterns "seeming" a bit inefficient, it's also requiring architectural changes to the target application to avoid unnecessary component reloads (similar to what @robbinjanssen stated). As a result, it seems there is no clean migration path from v5 to v6 and may roll back to v5 until more time can be invested to pull projects forward to v6.

I want to emphasize, the work on react router is valuable and much appreciated. I too wish there was a bit more collaborative discussion on this before such an impactful change.

@bthorben
Copy link

bthorben commented Nov 5, 2021

I would like to chime in and second that this is a huge change. It means changing the architecture of an application. For our application, this will result in multiple weeks of work.

@Kepro
Copy link

Kepro commented Nov 9, 2021

same here, our application will need couple of weeks of refactoring because of missing optional parameters :( I was very excited about v6 but looks like v5 is still better choice for our team now :(

@elliotdickison
Copy link

elliotdickison commented Nov 9, 2021

Really appreciate the work on react router. This comment is meant to be feedback, not a complaint, but I'm starting a new project on v5 because of this issue. It's possible I'm just not the target audience of v6 (I'm using react-router in combination with react-navigation for a cross-platform app, so I've got compatibility issues to think about). v6 looks super awesome and I love the approach, I'm just not sure I understand why optional parameters don't fit in the paradigm.

@vylink
Copy link

vylink commented Dec 5, 2021

Hi there, We are using dynamic routing(based on data from the server). The following path was simply perfect for our case.

<Route
  path="/:id?/:view?/:id2?/:view2?/:id3?/:view3?/:id4?/:view4?"
  render={() => <ReturnedComponent someParams={someParams}  />}
/>

Returned component decided what to do based on the optional parameters. Is there any way, how to achieve this kind of behaviour with V6?

@queengooborg
Copy link

I'm a bit surprised to see that support for optional parameters was dropped in v6. This is an extremely useful feature, and its removal has made a v5 to v6 project more challenging. The workarounds people here have suggested are great for smaller apps, but it's not clear why there has to be a workaround in the first place?

I'd love to hear the reasoning behind this change.

@sridharravanan
Copy link

@gopeter Yes, or you can do something like this:

<Route path="/page/:friendlyName">
  <Route path=":sort" element={<Page />} />
  <Route path="" element={<Page />} />
</Route>

its working

@Kepro
Copy link

Kepro commented Dec 9, 2021

@timdorr any reply that you want to add?

@jrmyio
Copy link

jrmyio commented Dec 10, 2021

I will give my take on this because I find it rather unfortunate that in 2021 the most popular Javascript router doesn't support a pattern I've been using successfully for almost 2 decades.

As far as I can see from reactions and discussions in this and other issues, the reason optional parameters are not supported in v6 is because they don't work well with the idea of the / (in the URL) representing a step deeper into the application. In the context of Remix, the pathname of the URL is mainly to follow the hierarchy of your page, for example:
User -> Messages > Inbox with the route user/messages/inbox.

When you want to use optional parameters, these are (almost) never related to the hierarchy of the page. Thus, it is suggested that you use query params. For example for ordering, filtering, pagination of the inbox you would start using the querystring:
user/messages/inbox?page=1&order=email.

However, folks at @remix reference the old PHP days quite a lot, and they might remember that before the nginxes and node servers, people used apache's .htaccess rewriterule to rewrite urls like /$order/$page => to something like index.php?order=$order&page=$page, in order to get those clean urls. There were, and still are, reasons why people do not want to use the querystring for these kind of things, here are some advantages you get when not using the query string:

  • You can have a single unique URL for each page:
    For example, using pokemon/fire/2 vs using a querystring, which will have multiple possible urls: pokemon?type=fire&page=2 and pokemon?page=2&type=fire. They may clutter your google index pages or prevent CDN caches when these params are not always in the same order.

  • Urls without using a querystring are shorter and look cleaner:
    inbox/1/email/desc vs inbox?page=1&order=email&order_direction=desc.

  • They are better for SEO:
    When optimizing for a keyword audi rs7, google likes cars/audi/rs7 more than cars?brand=audi&type=rs7 (atleast this was the consensus when I was still into SEO).

  • You can use the URL bar as a way of navigating:
    For example flights/amsterdam/2021-11-01/2021-11-03/2 is a pretty easy URL to grasp, it almost invites you to edit it manually instead of using that city selector or time widget to specify your search query, I think flights?from=2021-11-01&city=amsterdam&till=2021-11-03&page=2 is not all that inviting. Although this is more of a power user feature, I do find myself using this all the time where I implemented these kind of clean URLs.

Now, I haven't put much thought in how to get the best of both worlds, and I realize that by keeping a nested <Route/> about the hierarchy of the page helps a dev understand what a route really means, and gives frameworks like remix an easier time doing its magic with these pages.

That's why I think adding an additional component to deal with a part of the pathname that is not related to the hierarchy (optional parameters), might be worth exploring.

For example, Something like <QueryPath/> ? This would differentiate it from being a regular <Route/>, and tell react-router it is simply a configuration to map the query params to a nicer pathname. This part of the pathname is expected to come AFTER the base pathname of the current parent Route. It could potentially also configure default values for each of its optional parameters, for example:

<QueryPath path={':city/:from/:till/:page'} defaults={{ city: 'all', from: 'any', till: 'any', page: 1 }} />

a seperate hook like useQueryPath, or maybe just the useParams could return the values for these params. Values that are their default could be left out, or set to null.

Since constructing these URLs is pretty hard it would be great if one can reuse the configuration (the path={':city/:from/:till/:page'} defaults={{ city: 'all', from: 'any', till: 'any', page: 1 }}- part) with a function like generateQueryPath which could generate this part of the pathname, removing all the default values used at the end.

I hope this is somewhat convincing that there are use-cases and I hope it helps moving this discussion further 🙄 .

@n8sabes
Copy link

n8sabes commented Dec 10, 2021

Quick Optional Parameter Solution — Following up on prior post.

@jrmyio makes some good observations and worth the read. For the record, I love react-router v5. However, rearchitecting for v6 with what seem to be unintuitive code patterns (hacks) that necessitated deep app code/flow changes, simply to get existing value, required too much effort without meaningful new value.

After much head banging, hit a hard-stop with v6 and decided to try wouter. Ended up migrating to wouter quickly with very little effort as the wouter GitHub page is straight-forward, making it an easy migration guide to swap out react-router and regain optional parameters.

Hopefully there will be more time to spend with react-router v6, but for now wouter is meeting expectations, and is a smaller library. Keeping fingers crossed for optional parameter backward compatibility from v6!!

Maybe this issue should be re-opened?

@ymoon715
Copy link

This doesn't feel like a react-router 6. This feels more like a new router with a new philosophy.
I'm gonna have to stick to v5 and cross my fingers I find a worthy alternative. This isn't just a "breaking change". This is actually "must find alternatives" for a lot of people including me

@PeteJobi
Copy link

For those with the problem of pages unmounting and remounting on route change, I found out now that if you put your element on the parent <Route>, there will be no unmounts. That is, in the case of @robbinjanssen, if he changes the parent route

<Route path="tickets">
   ...
</Route>

to this:

<Route path="tickets" element={<List />}>
   ...
</Route>

It should work as he expects....

I can't say for sure though as this may depend on the implementation for <List/>, but it works for me.

@saidMounaim

This comment has been minimized.

@Kepro
Copy link

Kepro commented Mar 22, 2022

@timdorr so are we playing a dead bug? :) lot of downvotes, a lot of developers are in pain because you removed optional parameters without explanation or open discussion...

@timdorr
Copy link
Member

timdorr commented Mar 22, 2022

I didn't remove them personally. Don't be rude and direct your anger at me. I haven't contributed to the code base in some time.

@Kepro
Copy link

Kepro commented Mar 22, 2022

@timdorr, sorry but you were the one that wrote "We don't plan on supporting them in v6." without any explanation...

@caub
Copy link

caub commented Mar 22, 2022

@timdorr Can I try some pull-request for supporting them? in the same direction than my previous comment (it would expand all possibilities)

@jinspe
Copy link

jinspe commented Mar 27, 2022

another hacky solution if you don t want to unmount your components is to use useSearchParams so your url will look like this
/params1?p2=params2
you will have to do a bit of logic inside your components with a useEffect on the searchParams this can get kind of ugly but maybe it is easier that way.

@zachsiegel-capsida
Copy link

I agree with the frustrated commenters that this removal should absolutely be documented, since it used to be supported, and since many widely-used routers support and have historically supported optional parameters in a first-class way.

Or you could just support them.

Thank you!

@caub
Copy link

caub commented Mar 31, 2022

Here's a good workaround: https://replit.com/@caub/makeOptionalRoutes#index.js

EDIT: I start to understand this decision to not support optional path params, it's more explicit and actually better, I ended up just adding all possible routes in my config (and not using the helper above)

@actuallymentor
Copy link

@timdorr Can I try some pull-request for supporting them? in the same direction than my previous comment (it would expand all possibilities)

Not sure how much your bandwidth is, but I think many people would appreciate a pull request for that, and would argue in favor for that functionality.

Not supporting optional parameters is counter industry standards and many of us have headaches because of this choice.

@powelski
Copy link

I thought that optional params and dynamic params (ability to match multiple segments as a single param) were the most basic features of any reasonable router library. I was wrong.

@zachsiegel-capsida
Copy link

@powelski I wonder whether there's a good reason for this design choice, though. Maybe the idea was that processing route parameters should happen in functional components with their own (sub-) routes, and maybe optional parameters can always be handled in this way.

If anyone can drop some knowledge about this project, I would greatly appreciate it!

@powelski
Copy link

powelski commented Jun 21, 2022

@zachsiegel-capsida no idea, but to me it looks like a terrible decision.

@alexiuscrow
Copy link

I searched for a way to use the optional path prefix. In my case, it needed to split the routes by locales.
Taking into account the changes in the v6, there is a simple scratch with a better solution I found:

import MainContainer from '../MainContainer';
import {Routes, Route} from 'react-router-dom';
import HomePage from '../../pages/Home';
import AboutPage from '../../pages/About';
import NotFoundPage from '../../pages/NotFound';
import {useTranslation} from 'react-i18next';

const Application = () => {
  const {i18n} = useTranslation();
  const langPathPrefixes = [...i18n.languages, ''];

  const langRoutes = langPathPrefixes.map(langPrefix => (
    <Route path={`/${langPrefix}`} element={<MainContainer />} key={`react-router-${langPrefix}`}>
      <Route index element={<HomePage />} />
      <Route path="about" element={<AboutPage />} />
      <Route path="*" element={<NotFoundPage />} />
    </Route>
  ));

  return <Routes>{langRoutes}</Routes>;
};

export default Application;

@AndrewEastwood
Copy link

jamalkaksouri

seems more clunky and messy right now ...

@peterlau
Copy link

peterlau commented Aug 1, 2022

As an OSS maintainer (of a different project) myself I appreciate that when folks are contributing a useful product for free, it's a case of what-you-see-is-what-you-get.

In this instance, I think what's missing from the official response is that "We're not supporting optional params in V6 because nested Routes semantically more flexible".

The 'Whats new' docs do go into some depth about the changes: https://reactrouter.com/docs/en/v6/upgrading/v5#relative-routes-and-links

@TruePath
Copy link

TruePath commented Aug 10, 2022

So if I have routes like the following (e.g. the replacement for optional parameters) and just assign the same key prop to both won't react treat them as the same component (retaining state and keeping what it can in the DOM)?

        <Routes>
            <Route path="/" element={<BarWrapper />}>
                <Route path="*" element={<DefaultBar />}/>
                <Route path="project/:id/:action?/*" element={<ProjectBar key='projectbar'  />}/>
                <Route path="project/:id/*" element={<ProjectBar key='projectbar' />}/>
            </Route>
        </Routes>

If that's correct then it seems like this is a pretty easy fix. However, I'm not sure enough about react internals to be totally sure what this does and/or how it works with class components.

EDIT: Ok, not a super easy fix if you have an optional parameter below a parameter the key depends on but at least it's a fix.

Ahh, I now see why @alexiuscrow was suggesting the approach he did.

@AChristoff
Copy link

AChristoff commented Aug 15, 2022

I am sorry to say it but v5 is better than v6. Super cool features are gone... like gone!!! and why ??? Instead of adding more and make the old easy to use you guys cut so much of the router..... I mean who does that?!

@douglasrcjames
Copy link

@gopeter Yes, or you can do something like this:

<Route path="/page/:friendlyName">
  <Route path=":sort" element={<Page />} />
  <Route path="" element={<Page />} />
</Route>

For my use case of dynamically generated routes that need an optional parameter, the above solution does not work I do not think. See my use case below. I can't do this nesting trick because the productUrl must be set for the path in the parent Route, which if looped through with the map will produce multiple Route parents for each product. Any idea for a workaround for this? Seems ripe for optional usage. Pretty confused on the react-router's lack of reasoning to this decision to have no optional.

 {
    this.props.products.map((product, p) => {
        let productUrl = urlify(product.name);
        return (
            <Route 
                key={p}
                path={`${productUrl}`} 
                element={
                    <ErrorBoundary>
                        <ProductPage
                            product={product}
                            site={this.props.site} 
                        />
                    </ErrorBoundary>
                }
            />
        );
        
    })
}

@elbakerino
Copy link

@douglasrcjames

Depends how you can extract the "params" part from urlify and the info "does it have nested children" - and some other catchups if your URL structure is a chaos :D

With some comments here, this pattern works like a charm for me - also with automatically created routes - performance optimized.

  • /office is the full base path for all example routes
  • PageOffice is rendered only at exactly /office
  • /office/mails is the full base for the mails: showing list of mails AND when selected also details of one mail
  • PageMails is rendered only once and not remounted when the visitor switches between anything below /office/mails
  • PageMails is rendered with an empty param for "just the mails path"
  • PageMails is rendered with mailId as param for "the :mailId path"
  • inside PageMails further relative routes are existing, but this parent doesn't need to know the detailed routes
    • thus the /* pattern together with :mailId, to let it render for all routes matching /office/mails/:mailId/*
    • if you have multiple, separated components, you most likely need to add the exact "managed child routes" like :mailId/reply
<Routes>
    {/* .. other routes .. */}
    
   <Route path={'/office'}>
        <Route index element={<PageOffice/>}/>
        <Route path={'mails'} element={<PageMails/>}>
            <Route path={':mailId'}/>
            <Route path={':mailId/*'}/>
        </Route>
    </Route>

    {/* .. other routes .. */}
</Routes>

In PageMails i'm using the useParams hook to get the mailId param - the hook seems not-scoped-to-hierarchy in v6.

const params = useParams()
const mailId = params.mailId

@HappyZombies
Copy link

What a complete and total disaster

@iamunadike
Copy link

iamunadike commented Aug 30, 2022 via email

@powelski
Copy link

powelski commented Aug 30, 2022

@iamunadike faster how? Faster by micro-optimisation measures if anything. Matching routes happens so rarely and is always relatively fast. You won't tell me that you feel any difference. Saying that v6 is better is a joke.

@iamunadike
Copy link

iamunadike commented Aug 30, 2022 via email

@ymoon715
Copy link

ymoon715 commented Aug 30, 2022

It seems like this is the direction a lot of React framework is heading towards. Next.js uses very similar way of routing as the v6. Remix, framework created by the creator of react-router, also has a routing system that they provide that's quite similar. I'm guessing they are using react-router within Remix, so it made a lot of sense for them to go ahead and implement these changes to fit their framework better.

As the user base for Next and Remix grows, this type of routing is starting to become the "standard" almost. Web development is heading towards more strictness, and react-router seems to be going into the same direction.

Why should Remix-run maintain v5 when their own framework completely moved away from its philosophy of routing?

After being salty having to refactor large parts of my code base (you can see my earlier comment on this post on how salty I was), I'm not so salty anymore. You shouldn't be anymore either, since this will simply prepare us for the future.

@powelski
Copy link

@ymoon715 what's not strict about dynamic route segments or nesting? It's pain in the ass, if anything.

@powelski
Copy link

@iamunadike micro-optimisation means irrelevant optimisation. Assuming v6 is any faster (did someone actually check this?), they gave away many convenient features in favor of this irrelevant performance gain? React routing happens on the client side, so we don't even benefit from those micro gains ourselves as app owners. In reality there is zero sense to make routing faster like that, it was already faster than enough before.

@elbakerino
Copy link

elbakerino commented Aug 30, 2022

Not only for UI routing, the reason why I'm no longer salty, as the UI adjustments where the least ones for getting all code more future proof.

e.g. the OpenApi Spec specifies "path params are always required".

With the bit less flexibility, comes a lot of interoperability - and for the cases where you still need to do it otherways, workarounds are always available - but now always as last resort, without having the impact when doing it "correct".

It just isn't nice that this team here doesn't communicate their reasonings behind it (or just somewhere hidden in a PR), e.g. I imagine it could have a direct relation to the element={<Comp/>} rendering and the performance regarding that for the now nested routes logic.

@powelski
Copy link

@elbakerino can we leave performance out of this topic? I can't imagine client-side routing system that would have significant impact on performance. You guys need to stop confusing mental comfort with actual optimisation.

@chaance
Copy link
Collaborator

chaance commented Sep 2, 2022

I'm going to create a topic for this over in Discussions and lock the conversation here, as it's strayed a bit from the original point and devolved into venting. We understand that adopting to v6 requires some refactoring (it is a major release, after all!), and that's always something we need to consider carefully when making significant feature changes.

I suspect that adding this feature in v6 wouldn't work without breaking changes to the matching algorithm, but I could be wrong. A more targeted and focused discussion would be a good place to work through proposals while considering potential breakage and edge cases.

New discussion on optional URL params is here

@remix-run remix-run locked as too heated and limited conversation to collaborators Sep 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests