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

Match params are mixed with search using react-router-config #5662

Closed
amangeot opened this issue Oct 26, 2017 · 3 comments
Closed

Match params are mixed with search using react-router-config #5662

amangeot opened this issue Oct 26, 2017 · 3 comments

Comments

@amangeot
Copy link

amangeot commented Oct 26, 2017

Hello,
I have an issue on server with the value of matched params:

Rendering /items/10, match.params.id will equal 10 as expected
Though rendering /items/10?test=1, match.params.id equals 10?test=1, the search gets included in params.


Here is how I declare a route to be used with react-router-config:

{
    path: '/items/:id',
    exact: true,
    component: ItemsRedirect,
}

Here is how I retrieve and pass the match to my fetchData producing the results above, using react-router-config:

      const routes = makeRoutes(store, req)
      const branch = matchRoutes(routes, req.url)

      // Fetch data required by matched components
      const data = await Promise.all(branch.map(({ route, match }) => {
        const { fetchData } = route.component
        return fetchData instanceof Function ? fetchData(store, match) : Promise.resolve(null)
      }))

With "react-router-config": "1.0.0-beta.4",

@timdorr
Copy link
Member

timdorr commented Oct 26, 2017

We don't process query strings at all in 4.0. You would need to add an optional section to your path pattern to make that work, but it would be up to you to parse the query string further.

@timdorr timdorr closed this as completed Oct 26, 2017
@amangeot
Copy link
Author

amangeot commented Oct 27, 2017

When processing this url /items/10?test=1 based on the pattern /items/:id

1/ withRouter will give a match.params.id equal to 10 as expected,
2/ react-router-config will offer a match.params.id equal to 10?test=1

Is it intended to have a different match between matchRoutes and withRouter?

The match given to routes' component doesn't include the search either (same behaviour as withRouter).

@pshrmn
Copy link
Contributor

pshrmn commented Oct 27, 2017

RR uses path-to-regexp for route matching. path-to-regexp only does pathname matching. Instead of passing req.url (which contains the pathname, search, and hash segments), you should use req.path.

Alternatively, you should be creating a history object, in which case you could pass the pathname from the current location to matchRoutes.

const history = createMemoryHistory({ initialEntries: [req.url] });
matchRoutes(routes, history.location.pathname);

zdavis added a commit to ManifoldScholar/manifold that referenced this issue Nov 17, 2017
This change is made on the basis of this issue in React Router:

remix-run/react-router#5662.

By passing the entire URL to React Router for the server-side render,
we were running into cases where the route was not being matched as
expected. For example, if the route path was "/projects/:id" and the
url was http://manifold.dev/projects/some-project?foo=bar, the server-
side render would be passed a project param of `some-project?foo=bar`
rather than the expected `some-project`. This change solves that
problem, but I'm concerned that we're no longer passing any query
params from the URL to the server-side render, which may lead to mis-
matches between SSR and CSR.
zdavis added a commit to ManifoldScholar/manifold that referenced this issue Nov 17, 2017
This change is made on the basis of this issue in React Router:

remix-run/react-router#5662.

By passing the entire URL to React Router for the server-side render,
we were running into cases where the route was not being matched as
expected. For example, if the route path was "/projects/:id" and the
url was http://manifold.dev/projects/some-project?foo=bar, the server-
side render would be passed a project param of `some-project?foo=bar`
rather than the expected `some-project`. This change solves that
problem, but I'm concerned that we're no longer passing any query
params from the URL to the server-side render, which may lead to mis-
matches between SSR and CSR.
zdavis added a commit to ManifoldScholar/manifold that referenced this issue Nov 17, 2017
This change is made on the basis of this issue in React Router:

remix-run/react-router#5662.

By passing the entire URL to React Router for the server-side render,
we were running into cases where the route was not being matched as
expected. For example, if the route path was "/projects/:id" and the
url was http://manifold.dev/projects/some-project?foo=bar, the server-
side render would be passed a project param of `some-project?foo=bar`
rather than the expected `some-project`. This change solves that
problem, but I'm concerned that we're no longer passing any query
params from the URL to the server-side render, which may lead to mis-
matches between SSR and CSR.
zdavis added a commit to ManifoldScholar/manifold that referenced this issue Nov 17, 2017
This change is made on the basis of this issue in React Router:

remix-run/react-router#5662.

By passing the entire URL to React Router for the server-side render,
we were running into cases where the route was not being matched as
expected. For example, if the route path was "/projects/:id" and the
url was http://manifold.dev/projects/some-project?foo=bar, the server-
side render would be passed a project param of `some-project?foo=bar`
rather than the expected `some-project`. This change solves that
problem, but I'm concerned that we're no longer passing any query
params from the URL to the server-side render, which may lead to mis-
matches between SSR and CSR.
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
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

3 participants