Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Dynamic route not working with optional segment #6021

Closed
1 task done
Grandnainconnu opened this issue Apr 8, 2023 · 11 comments
Closed
1 task done

Dynamic route not working with optional segment #6021

Grandnainconnu opened this issue Apr 8, 2023 · 11 comments

Comments

@Grandnainconnu
Copy link

Grandnainconnu commented Apr 8, 2023

What version of Remix are you using?

1.15.0

Are all your remix dependencies & dev-dependencies using the same version?

  • Yes

Steps to Reproduce

https://codesandbox.io/p/sandbox/weathered-feather-kmfkgb

Expected Behavior

($lang)/$pid.tsx should match with and without the optional $lang parameter

Actual Behavior

The $pid.tsx route will only be accessible using /something/test
/test for example will not be matched

@Grandnainconnu Grandnainconnu changed the title Splat route not working with optional segent Splat route not working with optional segment Apr 8, 2023
@Grandnainconnu Grandnainconnu changed the title Splat route not working with optional segment Splat route not working with optional segment and layout route Apr 8, 2023
@Grandnainconnu Grandnainconnu changed the title Splat route not working with optional segment and layout route Splat route not working with optional segment Apr 8, 2023
@gijsroge
Copy link

gijsroge commented Apr 9, 2023

You should create a reproduction, I did this a few days ago, worked fine for me.

@Grandnainconnu
Copy link
Author

You should create a reproduction, I did this a few days ago, worked fine for me.

Here you go: https://codesandbox.io/p/sandbox/weathered-feather-kmfkgb?file=README.md

For some reason, I can't bump the version to 1.15, but the behavior appears to be the same.

@gijsroge
Copy link

gijsroge commented Apr 9, 2023

Misunderstood the question

But the behavior you are getting is normal.

/something -> resolves to ($language)/index.tsx because something is the language param, and index.tsx is the root of the ($lang) directory.
/something/test -> resolves to ($language)/$pid.tsx because something is the language param and test is the $pid param.

You can't expect the router to know that /something is not the language param. And as far as I know remix doesn't have support for regex optional params.

Also, you are referring to a splat route, but a spat route is something else.
For next time, just use https://remix.new to make a reproduction.

@Grandnainconnu
Copy link
Author

Grandnainconnu commented Apr 9, 2023

Misunderstood the question

But the behavior you are getting is normal.

/something -> resolves to ($language)/index.tsx because something is the language param, and index.tsx is the root of the ($lang) directory. /something/test -> resolves to ($language)/$pid.tsx because something is the language param and test is the $pid param.

You can't expect the router to know that /something is not the language param. And as far as I know remix doesn't have support for regex optional params.

Also, you are referring to a splat route, but a spat route is something else. For next time, just use https://remix.new to make a reproduction.

That's what I would have thought too, but take a look at the doc:
https://remix.run/docs/en/1.15.0/file-conventions/routes-files#optional-segments

Doc

My bad for the splat route, I changed to dynamic.

@Grandnainconnu Grandnainconnu changed the title Splat route not working with optional segment Dynamic route not working with optional segment Apr 9, 2023
@gijsroge
Copy link

gijsroge commented Apr 9, 2023

Ah, didn't see that. Yeah that's just faulty docs afaik.

@kiliman
Copy link
Collaborator

kiliman commented Apr 10, 2023

I'm not so sure. I think it's a bug in the implementation. My understanding is that optional segments are lazily matched. It will always try to eagerly match the route without the optional segments first.

When Remix sees a route with an optional segment, it generates multiple route definitions. Here's what it would look like, sorted by specificity.

/index
/category
/:pid
/:lang/index
/:lang/category
/:lang/:pid

@Grandnainconnu
Copy link
Author

I'm not so sure. I think it's a bug in the implementation. My understanding is that optional segments are lazily matched. It will always try to eagerly match the route without the optional segments first.

When Remix sees a route with an optional segment, it generates multiple route definitions. Here's what it would look like, sorted by specificity.

/index
/category
/:pid
/:lang/index
/:lang/category
/:lang/:pid

In which case, index would match then?

@kiliman
Copy link
Collaborator

kiliman commented Apr 10, 2023

I don't know if /:lang/index would ever match, since it would probably match /:pid first. It's possible that if you had a trailing slash, then /:lang/index may match.

That's why you need to be careful about ambiguous routes.

@Moucky
Copy link

Moucky commented May 1, 2023

I am experiencing this issue and would appreciate any help. Is it possible to have a single level dynamic routes with an optional segment? What about custom routing?

@pcattori
Copy link
Contributor

pcattori commented May 4, 2023

Related to #4857

@brophdawg11
Copy link
Contributor

I agree the docs are incorrect in that specific example.

This is behaving correctly at the moment because the index route carries more weight. This structure generates a routes tree of the following per npx remix routes

<Routes>
  <Route file="root.tsx">
    <Route path=":lang?" index file="routes/($lang)/index.tsx" />
    <Route path=":lang?/:pid" file="routes/($lang)/$pid.tsx" />
  </Route>
</Routes>

The implementation explodes these ahead of time to represent the routes with and without the optional segment, so the effective routing tree is:

<Routes>
  <Route file="root.tsx">
    <Route path=":lang" index file="routes/($lang)/index.tsx" />
    <Route path=":lang/:pid" file="routes/($lang)/$pid.tsx" />
    <Route path="" index file="routes/($lang)/index.tsx" />
    <Route path=":pid" file="routes/($lang)/$pid.tsx" />
  </Route>
</Routes>

And then when you only have one segment of the URL, path=":lang" index will rank higher than path=":pid" since the index aspect adds to the route scoring.

If you have no desire for /anything to ever match :lang then you don't actually want an index route inside of the ($lang) directory. You instead want:

routes/
  ($lang)/
    $pid.tsx
  index.tsx

Which will generate a route tree of:

<Routes>
  <Route file="root.tsx">
    <Route path=":lang?/:pid" file="routes/($lang)/$pid.tsx" />
    <Route index file="routes/index.tsx" />
  </Route>
</Routes>

Which explodes to the following which eliminates the ambiguity between path=":lang" and path=":pid":

<Routes>
  <Route file="root.tsx">
    <Route path=":lang/:pid" file="routes/($lang)/$pid.tsx" />
    <Route path=":pid" file="routes/($lang)/$pid.tsx" />
    <Route index file="routes/index.tsx" />
  </Route>
</Routes>

ramiroazar added a commit to ramiroazar/remix that referenced this issue Jul 18, 2023
…l segment

As outlined in the following issues, this route is not matched as stated in the documentation.

Whether this is a bug with remix instead still seems up for discussion.

I think this should actually work as currently documented, but it doesn't and this seems known for a while, so this change is just reflecting the current implementation.

remix-run#6021
remix-run#6317
remix-run#6426
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

6 participants