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

Dynamic folder routes ([...rest]/index.astro) take precedence over index.astro in 4.2.1 #9770

Closed
1 task done
mass8326 opened this issue Jan 22, 2024 · 6 comments · Fixed by #9786
Closed
1 task done
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) regression

Comments

@mass8326
Copy link

mass8326 commented Jan 22, 2024

Astro Info

Astro                    v4.2.1
Node                     v18.18.0
System                   Linux (x64)
Package Manager          unknown
Output                   server
Adapter                  none
Integrations             none

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

Similar to #9724.

While a route of [...rest].astro was fixed with #9726, a route of [...rest]/index.astro still overrides index.astro.

What's the expected result?

The index.astro page should be displayed rather than [...rest]/index.astro page. In the linked repro, the page should display "Index" but instead displays "Dynamic".

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-pqgx5b-azyze8?file=src%2Fpages%2F[...dynamic]%2Findex.astro

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Jan 22, 2024
@jaredLunde
Copy link

Yeah this regression exists in every release >=4.2.0

@lilnasy lilnasy added - P5: urgent Fix build-breaking bugs affecting most users, should be released ASAP (priority) regression - P3: minor bug An edge case that only affects very specific usage (priority) and removed needs triage Issue needs to be triaged - P5: urgent Fix build-breaking bugs affecting most users, should be released ASAP (priority) labels Jan 23, 2024
@lilnasy
Copy link
Contributor

lilnasy commented Jan 23, 2024

Reducing priority since a spread param in the middle of the path is unusual. We haven't officially supported it, explaining the absence of tests here.

Description of the use-case they enable would help.

@mass8326
Copy link
Author

mass8326 commented Jan 23, 2024

It's the way I've preferred to organize my projects, separating colocated components into different folders.

A src/pages structure might look like this. There's even an example in the Astro docs that is similar.

  • blog
    • _component-a.astro
    • _component-b.astro
    • index.astro
  • gallery
    • _component-a.astro
    • _component-b.astro
    • index.astro
  • _component-a.astro
  • _component-b.astro
  • index.astro

Due to this regression, I would need to organize in a flat structure like this:

  • _blog-component-a.astro
  • _blog-component-b.astro
  • _gallery-component-a.astro
  • _gallery-component-b.astro
  • _index-component-a.astro
  • _index-component-b.astro
  • blog.astro
  • gallery.astro
  • index.astro

Or by using hidden folders like this, which doesn't feel good:

  • _blog
    • component-a.astro
    • component-b.astro
  • _gallery
    • component-a.astro
    • component-b.astro
  • _index
    • component-a.astro
    • component-b.astro
  • blog.astro
  • gallery.astro
  • index.astro

I know that the Astro docs here suggest placing components in src/components. However, I would greatly appreciate being able to colocate components so that I don't have jump between different directories.

As evidence that I'm not the only one who prefers organizing like this, it would be similar to SvelteKit's filesystem routing. It's something which they believed in enough to make breaking changes to enforce. An excerpt from the link:

I frequently find myself moving foo.svelte to foo/index.svelte as the route becomes more complex (e.g. it needs a dedicated error page, or it gains a child route, or I need to break something out into a separate component, or it needs a page endpoint, and so on). These changes are costly and annoying, and I always kick myself for not just always using folders.

@ematipico
Copy link
Member

This isn't a similar issue. This is expected because [...dynamic]/index.astro has more segments than index.astro.

In your case, you should have a file called [...dynamic].astro.

From the docs:

Routes with more path segments will take precedence over less specific routes. In the example above, all routes under /posts/ take precedence over /[...slug].astro at the root.

@ematipico ematipico added needs response Issue needs response from OP and removed - P3: minor bug An edge case that only affects very specific usage (priority) regression labels Jan 23, 2024
@mass8326
Copy link
Author

@lilnasy
My bad, it seems like my initial examples didn't explain the use case for specifically using spread params in the middle of a route. Here's an updated example of a blog, where a page such as /blog/recipes/chicken-noodle-soup should take priority over /blog but does not. The points about colocation and folder organization I mentioned above would still apply.

  • blog
    • [...rest]
      • _component-a.astro
      • _component-b.astro
      • index.astro
    • _component-a.astro
    • _component-b.astro
    • index.astro
  • _component-a.astro
  • _component-b.astro
  • index.astro

@ematipico
My expectation was that [...dynamic]/index.astro would be treated the same as [...dynamic].astro. It seemed to work this way in 4.1.1 as seen here. It's also how my brain has been trained to understand Node's module resolution, where import { ... } from "./example" would work with either ./example/index.js or ./example.js.

I can understand that, if using spread params was not an approved or supported way of doing things, then it's technically not a regression. It would still be nice to be able to have this as a feature request, as it would make it much easier to colocate components with their routes and keep things organized.

I'd even argue that any fully static route (e.g. index.astro) should always be more specific that any route that has any dynamic component (e.g. [...rest]/index.astro). If it weren't more specific, why would a user create the fully static route in the first place?

@lilnasy lilnasy added - P4: important Violate documented behavior or significantly impacts performance (priority) regression and removed needs response Issue needs response from OP labels Jan 23, 2024
@lilnasy
Copy link
Contributor

lilnasy commented Jan 23, 2024

That's a very neat way to organize! Thanks for explaining it so well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants