The Wayback Machine - https://web.archive.org/web/20200601232149/https://github.com/nuxt/nuxt.js/issues/5874
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

Unable to create a dynamic route with required parameter by following the guide #5874

Open
Ingramz opened this issue Jun 5, 2019 — with CMTY · 13 comments
Open

Unable to create a dynamic route with required parameter by following the guide #5874

Ingramz opened this issue Jun 5, 2019 — with CMTY · 13 comments

Comments

Copy link
Contributor

@Ingramz Ingramz commented Jun 5, 2019 — with CMTY

Version

v2.8.1

Reproduction link

n/a

Steps to reproduce

Create following file/directory structure:

/pages/test1/_param1.vue
/pages/test2/_param2/index.vue

What is expected ?

Routes to be generated as:

/test1/:param1?
/test2/:param2

What is actually happening?

Routes are generated as:

/test1/:param1?
/test2/:param2?

Additional comments?

The guide says:

As you can see the route named users-id has the path :id? which makes it optional, if you want to make it required, create an index.vue file in the users/_id directory instead.

Following this advice (test2) results the parameter still being optional.

The test related to this functionality (

const files = [
'pages/index.vue',
'pages/_param.vue',
'pages/subpage/_param.vue',
'pages/snake_case_route.vue',
'pages/another_route/_id.vue',
'pages/another_route/_id.vue',
'pages/parent/index.vue',
'pages/parent/child/index.vue',
'pages/parent/child/test.vue'
]
) seems to cover only cases where the parameter is defined in a filename, eg being the very last node.

I believe that any parameters specified as directories should be treated as required, so it would be possible to have full control over whether a route parameter is required or optional without resorting to any workarounds.

This bug report is available on Nuxt community (#c9312)
@cmty cmty bot added the cmty:bug-report label Jun 5, 2019
@pimlie
Copy link
Member

@pimlie pimlie commented Jun 5, 2019

@Ingramz
Copy link
Contributor Author

@Ingramz Ingramz commented Jun 5, 2019

From what I can gather from this test is that one needs to create two routes, one for the intermediary path segment and other one for the parameter, like this:

/pages/test3/_param3.vue
/pages/test3/index.vue

But that would mean creating two routes. One that overrides the default case and rest will be handled by the parameter. Semantics wise this is ok as you'd normally expect the parent path tokens to always return something (however I don't exactly know if this is enforced by any sort of URI/HTTP specification).

If we ignore the semantics, creating two routes works around the problem, whereas a cleaner solution would be just one route that contains the required parameter part.

@pimlie
Copy link
Member

@pimlie pimlie commented Jun 5, 2019

Is it really an intermediate path though? If you need _param3 to be required, vue-router still needs to know what to do when _param3 was not supplied. Therefore you need to create index.vue which forces you to implement a solution for when _param3 is empty.

So required means here required for rendering the page component A. If the param was not supplied then you shouldnt use page component A but page component B because component A expect that param to eg do a API request.

@Ingramz
Copy link
Contributor Author

@Ingramz Ingramz commented Jun 5, 2019

But why introduce the overhead of component B? I am quite sure that vue-router is capable of deciding that /test3 should not match the route /test3/:param3 and in that case just display a 404.

@pimlie
Copy link
Member

@pimlie pimlie commented Jun 5, 2019

Thats true. To be honest I am not sure why it was created like the way it is

@pi0 @Atinux would you be ok with changing this behaviour or does it hold a deeper meaning?

@Ingramz
Copy link
Contributor Author

@Ingramz Ingramz commented Jun 5, 2019

Found the culprit, it's a documentation change :(

https://github.com/nuxt/docs/pull/888/files#diff-8017692b5218783a6438a7a1ed335c11

It looks like the two-file thing has been the intended pattern since the very beginning.

I personally would still like a change in behavior as it feels cleaner to me, but I fully respect if this is voted against.

@manniL
Copy link
Member

@manniL manniL commented Jun 7, 2019

@Ingramz what's the issue w/ the mentioned PR? 🤔

@Ingramz
Copy link
Contributor Author

@Ingramz Ingramz commented Jun 8, 2019

@manniL sorry for being too vague. Essentially adding the word instead to the end of the sentence completely changed the instructions.

As you can see the route named users-id has the path :id? which makes it optional, if you want to make it required, create an index.vue file in the users/_id directory.

First you had pages/users/_id.vue, now you need to also create pages/users/_id/index.vue, which turns :id required.

As you can see the route named users-id has the path :id? which makes it optional, if you want to make it required, create an index.vue file in the users/_id directory instead.

First you had pages/users/_id.vue, now you change it to pages/users/_id/index.vue, but it the parameter does not change to required.

@Ingramz
Copy link
Contributor Author

@Ingramz Ingramz commented Jun 8, 2019

Actually the first variant wouldn't work either because both files produce an equivalent result alone, the file needs to be pages/users/index.vue.

@stale
Copy link

@stale stale bot commented Jun 29, 2019

Thanks for your contribution to Nuxt.js!
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you would like this issue to remain open:

  1. Verify that you can still reproduce the issue in the latest version of nuxt-edge
  2. Comment the steps to reproduce it

Issues that are labeled as pending will not be automatically marked as stale.

@stale stale bot added the stale label Jun 29, 2019
@manniL
Copy link
Member

@manniL manniL commented Jun 30, 2019

Got it. @Ingramz would you mind to create a PR to fix it? ☺️

@stale stale bot removed the stale label Jun 30, 2019
@Ingramz
Copy link
Contributor Author

@Ingramz Ingramz commented Jul 1, 2019

I wouldn't mind, however I'll need to first understand whether my request of changing the behavior is reasonable compared to the way rest of the file to route mapping works.

If it turns out that is not a feasible option, I guess we can fix the documentation instead based on the knowledge gained from understanding the behavior better.

@lilianaziolek
Copy link

@lilianaziolek lilianaziolek commented Jul 12, 2019

Just came across this issue after tripping on documentation too... :)
If I may, +1 on changing code to behave as current doc says rather than changing doc to explain the double-file structure required for required params. I think that simply having a route with required param and relying on Vue Router's default behaviour (throwing 404 when no matching route is found) sounds much cleaner and intuitive.

Of course this kind of change would be potentially breaking so would require caution (maybe best suited for a major release?) - but that's definitely more in line with behaviour I'd expect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.