lambda-packages icon indicating copy to clipboard operation
lambda-packages copied to clipboard

🐛 BUG: Can't have 2+ dynamic routes in the same folder. Only one works

Open felixsanz opened this issue 3 years ago • 10 comments

What version of astro are you using?

v1.0.0-beta.42

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Linux

Describe the Bug

An example is worth more than 1000 words:

/pages/[page].astro -> Works https://91tzuw.sse.codesandbox.io/cookies

export async function getStaticPaths() {
	return [{
		params: { page: 'cookies' },
		props: {},
	}]
}

/pages/[post].astro -> Don't work, it gives 404 https://91tzuw.sse.codesandbox.io/post-1

export async function getStaticPaths() {
	return [{
		params: { post: 'post-1' },
		props: {},
	}]
}

According to documentation, it should work.

Link to Minimal Reproducible Example

https://codesandbox.io/s/vibrant-margulis-91tzuw

Participation

  • [ ] I am willing to submit a pull request for this issue.

felixsanz avatar Jun 16 '22 23:06 felixsanz

Triage opinion:

i think it's P4 or P5, a workaround would be convert dynamic route to static ones (P4), but not feasible with lot of routes (P5? lacking of workaround and core usage of astro). this issue goes against documented and expected behavior. Dynamic routing is one of the most important things in Astro. At least for me... I can't use astro anymore until this is fixed

felixsanz avatar Jun 20 '22 12:06 felixsanz

I don’t see it mentioned in the documentation.

It does mention a dynamic route and then a catch-the-rest route […]

readonlychild avatar Jun 20 '22 12:06 readonlychild

I don’t see it mentioned in the documentation.

It does mention a dynamic route and then a catch-the-rest route […]

1655729593

Ties = 2+ dynamic routes in the same level. Example: 2+ static routes, 2+ named parameters or 2+ rest parameters

felixsanz avatar Jun 20 '22 12:06 felixsanz

That is not what it says thou.

Albeit, I do not know what it means, you can have two same routes lake pages/create.astro and pages/[post].astro that resolved to create, but this is handled by the other rule, so it is not a tie.

Maybe create.astro vs create.html

readonlychild avatar Jun 20 '22 13:06 readonlychild

That is not what it says thou.

Albeit, I do not know what it means

If you do not know what it means, how do you know what it not says?

Maybe create.astro vs create.html

This makes no sense as it will be a conflict in build, both trying to output create.html. That's not a tie in priority order, it's just a duplication and i don't see people using .html in pages/

felixsanz avatar Jun 20 '22 13:06 felixsanz

How would this work in SSR? We have to know which route to pick before running it.

matthewp avatar Jun 20 '22 14:06 matthewp

How would this work in SSR? We have to know which route to pick before running it.

That's interesting. I think 2+ dynamic routes was supported time ago and that's the reason the documentation talks about ties in priority order (even aFuzzyBear was surprised about this thing broken, when he tried to solve my question in the support thread).

So i think this broke when SSR was introduced and no one noticed. How would this work in SSR? You got me 😆. Inside this docs page, the example handles 404 for missing products, which is impossible having 2+ dynamic routes (having [products].astro and [posts].astro... 404 for what page? product doesn't? blog post inexistent?).

Of course having just a blog or having just a store is too rigid to spot this issue. But what if you have a portfolio page with some pages about [talks].astro and some others about [posts].astro? You either sacrifice your routes and do /talks/* or/and /posts/* (moving one or all to subfolders), or you have to move all your talks/posts into static pages. imaginte having 50 static pages with your 50 talks because you can't have a dynamic route as [posts].astro is occupying it.

I think core has to discuss this and clear the future, as it was working before and now it doesn't and I think it's just impossible to make this work with SSR.

Solution A: Make it work again with SSG and state that this will not work with SSR routes. Solution B: Change docs, remove the priority tie thing and make a claim about 2+ dynamic routes with the same priority order not supported. Solution C: Summon the Astro developer gods so they come with a solution and make it work with SSG and SSR.

I'd prefer C or A, as B will make me sad (will need to convert a dynamic route into a bunch of static ones).

felixsanz avatar Jun 20 '22 15:06 felixsanz

Why can't you just move the logic for those dynamic routes into separate components, then have 1 dynamic route that defers to the right component?

/pages/[name].astro

---
import Page from '../components/Page.astro';
import Post from '../components/Post.astro';

export async function getStaticPaths() {
	return [{
		params: { name: 'cookies' },
		props: {type: 'page'},
	}, {
		params: { name: 'post-1' },
		props: {type: 'post'},
	}]
}

---
{ Astro.props.type === 'post' ? <Post {...Astro.props} /> : <Page {...Astro.props} /> }

Something like that.

matthewp avatar Jun 20 '22 15:06 matthewp

Why can't you just move the logic for those dynamic routes into separate components, then have 1 dynamic route that defers to the right component?

/pages/[name].astro

---
import Page from '../components/Page.astro';
import Post from '../components/Post.astro';

export async function getStaticPaths() {
	return [{
		params: { name: 'cookies' },
		props: {type: 'page'},
	}, {
		params: { name: 'post-1' },
		props: {type: 'post'},
	}]
}

---
{ Astro.props.type === 'post' ? <Post {...Astro.props} /> : <Page {...Astro.props} /> }

Something like that.

That's a good workaround but it isn't too hacky? I like setting my <head> stuff inside the /pages/*.astro files, and setting all visual content inside my components, but here i'm forced to have almost no code inside /pages/*, just the getStaticPaths and the conditional.

But yeah, i'm happy with the workaround and will use it (thanks!), just wondering if that's the best solution for the project? (if yes, then documentation needs an update)

Also.. this solution is even more hacky/dirty on SSR don't you think?

felixsanz avatar Jun 20 '22 15:06 felixsanz

I wouldn't really consider this to be a workaround at all. For any router it needs to know which route to pick based only on the URL. I'm pretty sure you cannot construct 2 routes that have the same definition in Express.js or Fastify or any of those types of frameworks.

I think what you can do in those types of projects is create a nested router. I believe Next.js has an RFC for nested routes, so maybe that's something we should pursue. That would be done via https://github.com/withastro/rfcs

matthewp avatar Jun 20 '22 15:06 matthewp