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

💡 RFC: File-Based Routing /w Dynamic Paths #1004

Closed
FredKSchott opened this issue Aug 4, 2021 · 19 comments · Fixed by #1010
Closed

💡 RFC: File-Based Routing /w Dynamic Paths #1004

FredKSchott opened this issue Aug 4, 2021 · 19 comments · Fixed by #1010

Comments

@FredKSchott
Copy link
Member

FredKSchott commented Aug 4, 2021

Try it out today: npm install astro@next--routing

Background

  • When we launched Astro, we implemented simple file-based routing for static pages.
  • We avoided more advanced file-based routing (ex: Next.js, SvelteKit) in the early days of the project because without hands-on experience, we were worried that we would design it poorly or in a way that made dynamic routing harder to build later on.
  • Instead, we shipped a feature called Collections which offered some dynamic support.

Motivation

  • Collections worked, but we still have trouble documenting and explaining this concept (most recently, for example, 📘 DOC: Better "Collections" examples and explanations, especially for those coming from other SSGs #973).
  • The multiple things that you need to configure (route, paths, and props) adds friction, since it really only works if all are defined.
  • Defining a route in config feels especially odd, because the file path itself is meant to be the route.
  • We've tried improving this in a previous PR, but we still see user pain around Collections due to how unique a concept this is and how difficult it can be to communicate well.

Now that Astro has launched publicly and is moving closer to a v1.0 beta, our biggest reasons to avoid building more advanced file-based routing are no longer relevant. This proposal is to add true file-based routing with dynamic path support, ala Next.js or SvelteKit.

Non-Goals

This RFC does not add dynamic, server-side routes to Astro. Astro is still a 100% static site builder. However, this RFC does solve for one of the biggest missing features in Astro that was blocking dynamic routing, so you can consider this as one step closer to unblocking dynamic routes in Astro.

Proposal

Filename Syntax

  • pages/blog/[slug].astro/blog/:slug (/blog/hello-world, /blog/post-2, etc.)
  • pages/[username]/settings.astro → (/fred/settings, /drew/settings, etc.)
  • pages/[lang]-[version]/info.astro → (/en-v1/info, /fr-v2/info, etc.)
  • pages/post/[...slug].astro → (/post/a, /post/a/b, /post/a/b/c, etc.)

getStaticPaths()

// Example: src/pages/post/[name].astro
export async function getStaticPaths() {
  return [
    { params: { name: 'post-1' } }, // generates "/post/post-1"
    { params: { name: 'post-2' } }, // generates "/post/post-2"
    { params: { name: 'post-3' } }, // generates "/post/post-3"
  ];
}
// Example: src/pages/post/[id].astro
export async function getStaticPaths() {
  // Fetch a collection of posts
  const data = await fetch('...').then(response => response.json());
  // Generate a page for every post, passing the full object as a prop
  return data.map((post) => ({params: { id: post.id }, props: { post } }));
}

Design

Proof of Concept PR with design & implementation details: #1010

Open Questions

  • Should the rest params be an array, or a string? For example: /a/b/c matching src/pages/[...id].astro could return a path object of {params: {id: 'a/b/c'}} or {params: {id: ['a', 'b', 'c']}. The user could easily map between the two as needed via split('/') and join('/') so this is really a question of "what's the best default?" FYI SvelteKit uses a string while Next.js uses an array.
  • When to merge this? We just refactored the Collections API so I'm hesitant to do another breaking change here. Maybe merge this in with the new compiler work, so that we can combine a breaking change with a really cool improvement?
@FredKSchott
Copy link
Member Author

Added label to discuss at our next RFC meeting!

@snowpackjs/contributors-l2 @snowpackjs/contributors-core I'd love to get consensus on the full design / implementation if possible with the intention of merging the PR if consensus is reached. If you expect to have feedback on this proposal please take a look ahead of time and share any thoughts here!

@brycewray
Copy link
Contributor

Very much looking forward to when Astro.fetchContent() can be used with more than just .md files.

Or, if that’s not possible, the return of components in Markdown.

Or, better yet, both. 😄 🙏

@jasikpark
Copy link
Contributor

jasikpark commented Aug 7, 2021

I think you can do something similar with regular imports? https://docs.astro.build/guides/imports/

Would

const data = import.meta.globEager(**/*.json); 

be valid?

from https://vitejs.dev/guide/features.html#json

@brycewray
Copy link
Contributor

Main interest is in dealing with existing Markdown files with imported images that, in other SSGs, I use shortcodes to handle responsiveness and such. With .astro files, would do with components, but can't do it in .md files — now.

@brycewray
Copy link
Contributor

Would also love to see documentation on exposed “next page,” “this page,” and “previous page” vars from this method (if they exist). To clarify, I refer not to such vars for a paginated list — already documented in the new API page — but, rather, for individual pages in a sorted list. That way, for example, each blog post in a sorted list can have “previous post” and “next post” vars (URL, data from front matter, etc.) to which it can link. If those are already exposed in Astro.props somehow, I apologize for noting this but I haven't yet seen the correct names for such vars.

@jasikpark
Copy link
Contributor

jasikpark commented Aug 7, 2021

I think you can find them here.  https://astro-docs-git-wip-fbr-pikapkg.vercel.app/reference/api-reference#paginate i misread

@brycewray
Copy link
Contributor

brycewray commented Aug 7, 2021

I thought those are for only pagination and work only with pages with the [page].astro or [...page].astro types of file names. Guess what I'm asking for is something like what Eleventy provides for every page (including from .md files) on a per-page basis as noted here.

@jasikpark
Copy link
Contributor

ah, so you want to access the pagination info from the individual post pages, that makes sense, there's a similar setup in Hugo .Page.Next and .Page.Prev.

@brycewray
Copy link
Contributor

brycewray commented Aug 7, 2021

Exactly. On my current site (https://www.brycewray.com), each post has a section at the bottom which uses the Eleventy collection to get previous post and next post. Now, mind you, that part wasn't always easy 😄 in Eleventy:
11ty/eleventy#529
https://www.brycewray.com/posts/2019/12/previous-next-eleventy/

For example, in my current Eleventy site, I have this at the bottom of my per-post template (this is Nunjucks)

  {% if page.url != "/about/" %}
  <div class="w-full px-8 md:px-0 bg-black dark:bg-blue-900 align-middle mt-10 mb-10">
    <h3 class="text-center text-3xl tracking-normal mb-2 pt-2"><a href="/posts" class="border-transparent text-blue-100 hover:text-blue-200 hover:border-blue-200">Other posts</a></h3>
  {% if nextPost.url %}
    <p class="font-sans text-center mt-2 mb-2 text-xl lg:text-2xl text-white leading-tight tracking-tight">
      <strong>Next</strong>: 
      <a class="border-transparent text-blue-100 hover:text-blue-200 hover:border-blue-200" href="{{ nextPost.url }}">{{ nextPost.data.title }}</a>
    </p>
  {% endif %}
  {% if prevPost.url %}
    <p class="font-sans text-center pb-4 mt-2 mb-0 text-xl lg:text-2xl text-white leading-tight tracking-tight">
      <strong>Previous</strong>: 
      <a class="border-transparent text-blue-100 hover:text-blue-200 hover:border-blue-200" href="{{ prevPost.url }}">{{ prevPost.data.title }}</a>
    </p>
  {% else %}
    <p class="p-0 -mt-2 leading-none">&nbsp;</p>
  {% endif %}
  </div>
  {% endif %}

@FredKSchott
Copy link
Member Author

Nice! I really like that. You can do the same in Astro using Astro.fetchContent() by fetching your collection in the page component script (or for markdown files, the layout component script) and then creating const nextPage = collection.find(...) and const prevPage = collection.find(...) yourself in that component script. Just make sure that you sort and filter the collection by tag, date, or whatever else you'd like.

@brycewray
Copy link
Contributor

Thanks @FredKSchott! (Although I thought the collection stuff went away in next--routing?) In the meantime, I'll just note that the most current version of next--routing (per your comment) no longer works with the pagination page prop (https://astro-docs-git-wip-fbr-pikapkg.vercel.app/reference/api-reference#paginate). Perhaps desired behavior, but just FYI. cc: @jasikpark

@FredKSchott
Copy link
Member Author

@brycewray sorry, not sure I follow your last comment. The page prop is broken in the latest version of next--routing?

@brycewray
Copy link
Contributor

brycewray commented Aug 9, 2021

@FredKSchott all I know is that the following, in /src/pages/posts/[...page].astro, worked in the previous version of next--routing...

---
export async function getStaticPaths({ paginate }) {
  const allPosts = Astro.fetchContent('../posts/**/*.md')
  const sortedPosts = allPosts.sort(
    (a, b) => new Date(b.date) - new Date(a.date)
  );
  // Return a paginated collection of paths for all posts
  return paginate(sortedPosts, { pageSize: 5 })
}
// The page prop has everything that you need to render a single page in the collection.
const { page } = Astro.props
import Layout from 'layouts/Base.astro'
import { zonedTimeToUtc, utcToZonedTime, format } from "date-fns"
const dateFormat = (item) => format((Date.parse(item)), 'MMMM d, yyyy')
---

<!DOCTYPE html>
<Layout
  title="Posts list"
>
  <main class="pt-12">
    <div class="sm:w-5/6 md:w-4/5 lg:2/3 xl:w-7/12 mt-10 mr-auto ml-auto px-6 lg:px-16">
      <article class="article">
        <h1>Testing the new routing stuff</h1>
        <p>This is just text.</p>
        <p>This is page number {page.page.current}.</p>
        <p>The total number of pages is {page.page.last}.</p>
        <p>Page URL = {page.url.current}</p>
        <p>{page.start + 1}{page.end + 1} of {page.total}</p>
        {page.data.map((post => 
          <>
          <p><a href={post.url}>{post.title}</a></p>
          <p>Date: {dateFormat(post.date)}</p>
          </>
        ))}
      </article>
      <p class="text-center">
        {page.url.prev 
          ? `<a href=${page.url.prev}>Previous</a>`
          : `<span class="text-gray-500">Previous</span>`
        }
      &nbsp;&nbsp;&bull;&nbsp;&nbsp;
        {page.url.next
          ? `<a href=${page.url.next}>Next</a>`
          : `<span class="text-gray-500">Next</span>`
        }
      </p>
    </div>
  </main>
</Layout>

...but now it produces the following when I try to view http://localhost:3000/posts:

[access] /posts/
[executing astro] TypeError: Cannot read property 'current' of undefined
    at Object.__render (/_astro/src/pages/posts/[...page].astro.js:80:906)
    at Object.__renderPage (/_astro/src/pages/posts/[...page].astro.js:114:46)
    at load (file:///Users/thewrays/astro-site/node_modules/astro/dist/runtime.js:110:34)
    at async Server.<anonymous> (file:///Users/thewrays/astro-site/node_modules/astro/dist/dev.js:21:20)
[access] /500

It also errors out during builds, so it's not just a dev-mode issue:

[build] Unable to render src/pages/posts/[...page].astro

TypeError: Cannot read property 'current' of undefined
    at file:///Users/thewrays/astro-site/src/pages/posts/[...page].astro
    at file:///Users/thewrays/astro-site/src/pages/posts/[...page].astro
    at Object.__renderPage (file:///Users/thewrays/astro-site/src/pages/posts/[...page].astro
    at load (file:///Users/thewrays/astro-site/node_modules/astro/dist/runtime.js:110:34)
    at async buildStaticPage (file:///Users/thewrays/astro-site/node_modules/astro/dist/build/page.js:25:18)
    at async Promise.all (index 0)
    at async file:///Users/thewrays/astro-site/node_modules/astro/dist/build.js:75:9
    at async Promise.all (index 9)
    at async build (file:///Users/thewrays/astro-site/node_modules/astro/dist/build.js:74:7)
    at async buildAndExit (file:///Users/thewrays/astro-site/node_modules/astro/dist/cli.js:10:15)

[build] ✕ building pages failed!

Still same repo — https://github.com/brycewray/astro-site — but, as noted, can't re-build the demo on Vercel right now, so what you'll see in https://astro-site.vercel.app/posts/ reflects what I got before.

@FredKSchott
Copy link
Member Author

Ah right, the interface has changed a bit. we can add a warning if you try to access page.page which no longer exists. It is now page.currentPage and page.lastPage: https://astro-docs-git-wip-fbr-pikapkg.vercel.app/reference/api-reference#paginate

@brycewray
Copy link
Contributor

Thanks! Will have to wait a while before I can try it, however -- my daughter is in the process of delivering our second grandchild. 😃

@FredKSchott
Copy link
Member Author

CONGRATULATIONS!!! That's amazing!!!

@openidauthority
Copy link

Would [optional_param?].astro be more intuitive than [...optional_param].astro?

Also, it seems that any change that affects the value returned by getStaticPaths() requires the dev server to be restarted. It would be nice if a change to that function would cause the dynamic routes to be refreshed.

@FredKSchott
Copy link
Member Author

Would [optional_param?].astro be more intuitive than [...optional_param].astro?

Yea, I wonder why Next.js and SvelteKit haven't implemented this. Does anyone know? I'd be +1 for adding this as a follow-up PR if it makes sense.

Because getStaticPaths() is responsible for creating your paths (the user just matches created paths), there isn't a huge difference between the two. You would return {params: {optional_param: undefined}} in both cases.

Also, it seems that any change that affects the value returned by getStaticPaths() requires the dev server to be restarted. It would be nice if a change to that function would cause the dynamic routes to be refreshed.

Yup, this regression is caused by the Snowpack bug linked in OP, fixed and ready to go out.

@openidauthority
Copy link

Would [optional_param?].astro be more intuitive than [...optional_param].astro?

Yea, I wonder why Next.js and SvelteKit haven't implemented this. Does anyone know? I'd be +1 for adding this as a follow-up PR if it makes sense.

On second thought, [...path] does make more sense because it matches the entire remainder of the path, not just a single segment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants