🐛 BUG: Conflict between pagination and dynamic routes in the same path

pyronaur commented 3 years ago

What package manager are you using?

npm (Node v16.9.1)

What operating system are you using?

Mac

Describe the Bug

The Problem:

Pagination doesn't work when [page].astro or [...page.astro] are used in the same directory as [slug].astro.

For example, fetching placing these 2 files in a /src/pages/pokemon/ directory, here's the result:

✅ /pokemon- Displays first page of pokemon
✅ /pokemon/bulbasaur - Shows a single pokemon
❌ /pokemon/2 - Pagination doesn't work
❌ /pokemon/3 - Pagination doesn't work

I've tried multiple ways of doing this and it seems that there's no easy way around this issue.

The ideal solution

It'd be great to have pagination work like in the example above.

Alternative Solution

Alternatively, perhaps allow customization of thepaginate() function - by allowing to pass a parameter how to rewrite the pages. For example:

paginate( allPokemon, { pageSize: 10, rewrite: 'paginated' });

/pokemon- Displays first page of pokemon
/pokemon/bulbasaur - Shows a single pokemon
/pokemon/paginated/2 - Second page of pokemon
/pokemon/paginated/3 - Third page of pokemon

Workaround

It is possible to workaround towards the "Alternative Solution", like so: https://github.com/pyronaur/astro-test-case/tree/master/src/pages/pokemon-workaround

But there are a few caveats:

Not as easy as just using Astro
It's a bit hacky, especially if you want to have a full pagination ( like <-- 1 2 3 4 --> ) - the index.astro file doesn't allow to use paginate() function, so anything that goes into index is going to be difficult to maintain
Other Astro tools and packages might not play well with a custom work-around solution, for example SEO, etc.

Steps to Reproduce

Clone the bug demo repository: https://github.com/pyronaur/astro-test-case
Run npm i && npm run dev
Open localhost:3000 and browse the test cases

Link to Minimal Reproducible Example (Optional)

https://github.com/pyronaur/astro-test-case

matthewp commented 3 years ago

Ah, yes, this is a logical problem we have with how dynamic routes work. Going to page @FredKSchott since he's been the champion of dynamic routes. Is this a flaw in the design or just a bug?

FredKSchott commented 3 years ago

Thanks for filing this! I agree, this should work. Can you move the [...page.astro] file to another subdirectory in the meantime? Something like: src/pages/pokemon/all/[...page].astro?

FredKSchott commented 3 years ago

Actually, the more I think about this the less confident I am that this should work. I'm not sure if its advised to put a [slug].astro and a [...slug.astro] in the same directory.

It would probably be fine for building, but how would the dev server know which route to match? Would we say that single-part URLs like /a always match [slug.astro] and multi-part URLs like /a/b always match [...slug].astro?

pyronaur commented 3 years ago

I'm not sure if its advised to put a [slug].astro and a [...slug.astro] in the same directory.

@FredKSchott You mean [slug].astro and [...page].astro ? Afaik, there's no ...slug so I'm slightly confused 😅

Do a quick clone of https://github.com/pyronaur/astro-test-case - I've created these 3 different interactive test cases to help you reason about it.

IanVS commented 3 years ago

As far as I know, @pyronaur, the names themselves do not really matter, you could call them [...foobar].astro.

I hit this issue as well recently, and it kind of makes sense that there's no real way for astro to differentiate between bulbasaur and 2 in dev. Although maybe Astro could try all the files with brackets until it finds one that works?

Maybe for now you could change your paginated routes to /pokemon/page/2? I think that's less ambiguous what the 2 represents in any case.

jasikpark commented 3 years ago

Actually, the more I think about this the less confident I am that this should work. I'm not sure if its advised to put a [slug].astro and a [...slug.astro] in the same directory.

It would probably be fine for building, but how would the dev server know which route to match? Would we say that single-part URLs like /a always match [slug.astro] and multi-part URLs like /a/b always match [...slug].astro?

https://docs.astro.build/core-concepts/routing/#caveats leads me to believe that there should be fallback, where the high specificity of dynamic route wins: so /a specified in /pages/[slug].astro would win over /a specified in /pages/[...slug].astro.

jasikpark commented 2 years ago

This is still the same behavior as of v0.21.12: https://stackblitz.com/edit/github-dpcsfs?file=src/pages/index.astro

I still think that allowing lower specificity to be overridden by higher specificity routes would be the way to go -- is that something that is simple to implement or would it require an RFC about how to resolve these conflicts?

If we decide that it's not allowed, we should at least have a specific error message for this case where you have multiple [foo].astro or [...bar].astro routes in the same place

pyronaur commented 2 years ago

I agree @jasikpark - I think it should be allowed, and if it isn't - a clear error message should be displayed.

But I think allowing overrides is pretty important. There are always exceptions in how people want to set up their sites. Every "non-static" architecture out there gives you a lot of flexibility - people coming from that side of things have come to expect that level of control.

Maybe for now you could change your paginated routes to /pokemon/page/2? I think that's less ambiguous what the 2 represents in any case.

@IanVS Check out the repository - I've listed 3 ways of doing this, two of which are broken. What you're suggesting is the workaround - and while it works - I'm forced into maintaining 2 separate files to achieve pagination. Imho - that's still a workaround, not proper pagination solution out of the box.

At the very least - if this specific pagination type is a documented way of "how astro does things" - it should handle errors and provide a way to set it all up from a single file.

jonathantneal commented 2 years ago

@jasikpark, do you think priority routing should be an RFC? @FredKSchott, are you looking to champion this further?

We don’t want older issues getting lost in the weeds, so I’d love to know how we can move forward or move on from the original issue.

jasikpark commented 2 years ago

I'm unsure whether it needs an RFC, but I would like to see this behavior in Astro, especially since it's already described as this way in the way I'm reading the docs (https://docs.astro.build/en/core-concepts/routing/#caveats)

FredKSchott commented 2 years ago

Unfortunately I just don't think that this will work. Neither Next.js or SvelteKit support it either. The reason is that you would need to try and run different routes to make a match. Right now, we use a much quicker regex-based match based only on the route and the filename.

The other tricky bit here is that we also want to support SSR, which means running a dynamic route for all URLs in production, on each request. In that case, we would only have access to match the route URL with the filename, similar to today's implementation.

Note that /foo.astro and /[slug].astro will still work! explicit static URLs like foo.astro will always take precedence over dynamic routes.

pyronaur commented 2 years ago

Neither Next.js or SvelteKit support it either

WordPress does, and it's slightly bigger than both of those projects combined. Chossing the competition you compare Astro to can accidentally limit the potential of the project 😉

This may not be a standard in the JS part of the world, but it would be really nice if with some configuration I could explicitly tell Astro what I want my routes to look like.

My reasoning for this is selfish, so I understand if this is going to become a wontfix - I want to migrate my WordPress blog to Astro, match the URLs exactly and then write a blog post about it and the performance gains. But that post loses steam if I have to explain that you'll have to lose SEO value of your previous posts and/or figure out redirects in order to move to Astro.

FredKSchott commented 2 years ago

Fair enough! Mind sharing some docs on how this works with Wordpress?

pyronaur commented 2 years ago

The docs are here, but I wouldn't say they're extremely helpful to explain this: https://wordpress.org/support/article/using-permalinks/

WordPress is doing a lot of lifting to figure out which page the user is trying to look at. Most of the code lives here, but that might be a bit much to look at because of the various other types of link structures that WordPress supports:

https://github.com/WordPress/WordPress/blob/master/wp-includes/rewrite.php https://github.com/WordPress/WordPress/blob/master/wp-includes/class-wp-rewrite.php https://github.com/WordPress/WordPress/blob/e840d56df528506e14fc014888596335efa92915/wp-includes/class-wp-query.php#L3556-L3621

In the WordPress ecosystem - this has become a de facto standard. Some larger sites (like TechCrunch) deviate from the standard plain permalinks structure slightly ( example.com/post-name is instead techcrunch.com/2022/01/post-name), but for the most part - posts and pages are all considered "site root level" entries.

The TL;DR is that you can configure WordPress permalinks to be example.com/%postname% which is going to try to make everything a root-level permalink, no matter whether it's a post, a page or anything else.

Here's a typical medium-sized WordPress site:

Single blog post: https://wptavern.com/automattics-livro-is-a-minimal-block-theme-for-writers

A blog post category: https://wptavern.com/category/wordcamps-meetups

Custom post type "Podcast" https://wptavern.com/podcast

"Podcast" post type has a paginated archive: https://wptavern.com/podcast/page/3

And a single podcast episode is here: https://wptavern.com/podcast/10-whats-in-wordpress-5-9-and-what-is-openverse

It also seems that Podcasts used to be just posts with a podcast category, but that evolved into it's own post type probably, which explains why this is a podcast, but doesn't show /podcast/ in the URL: https://wptavern.com/wpweekly-episode-362-fitness-freelancing-and-more-with-michelle-schulp

pyronaur commented 2 years ago

I think using WordPress source for inspiration might be too complex though. I just remembered that Elder.js also took inspiration from WordPress URL structure, so probably worth looking at this: https://elderguide.com/tech/elderjs/#routes

Also, I don't think Astro is too far from achieving this

One idea would be to have a custom routes file that takes up the responsibility for helping Astro decide where to look for the content generation file, something like /src/pages/my-custom-route/[router].astro

export function router( pathname ) {
    const path = pathname.replace( '/my-custom-route/', '' );
    if ( path.includes( 'podcast' ) ) {
        return '[podcast].astro';
    }

    if ( ! isNaN( path ) ) {
        return '[paginated-post].astro';
    }

    return '[post].astro';
}

console.log( router( '/my-custom-route/path/to/post' ) );
// > [post].astro

console.log( router( '/my-custom-route/2' ) );
// >[paginated-post].astro

console.log( router( '/my-custom-route/podcast-4' ) );
// >[podcast].astro

getStaticPaths method is already there that allows me to pass any data I want to astro. What I think it needs is a complimenting function that also defines how to "receive" the data that I've passed to Astro.

FredKSchott commented 2 years ago

I like the idea of a router, and I like the idea of adding some additional config/code-based routing on top of the default file-based routing. Thanks for sharing these!

matthewp commented 2 years ago

It looks like there isn't anything actionable here that we can do in the short-term. A new feature to handle this case needs an RFC. If you'd like to champion this feature, please do so here: https://github.com/withastro/rfcs

thdoan commented 1 year ago

I recently encountered a scenario similar to this, and this pattern has worked out really well:

To use the Pokemon example, it would be:

pages/
├─ pokemon/
│  ├─ [name]/
│  │  ├─ [...page].astro
│  ├─ [...page].astro

danilopaulinodasilva commented 7 months ago

I recently encountered a scenario similar to this, and this pattern has worked out really well:

To use the Pokemon example, it would be:
pages/
├─ pokemon/
│  ├─ [name]/
│  │  ├─ [...page].astro
│  ├─ [...page].astro

Can you share the code to achieve this? I am struggling from weeks trying to achieve this.

withastro / astro