WebDevStudios / nextjs-wordpress-starter

A headless starter for WordPress powered by Next.js.
https://webdevstudios.github.io/nextjs-wordpress-starter/
GNU General Public License v2.0
738 stars 129 forks source link

GetStaticPaths should not be fetching 10,000 nodes #1008

Open jasonbahl opened 2 years ago

jasonbahl commented 2 years ago

Hello πŸ‘‹πŸ»

I've been helping several people debug issues with using Next + WPGraphQL and it turns out one of the biggest issues I've been seeing is that folks are fetching thousands of nodes from WordPress and passing those nodes as paths in getStaticPaths.

This causes Next to build ALL pages every time a build is run and this leads to the WordPress server being hammered and sometimes becoming unresponsive.

We've had a lot of good success using Next ISR and passing only a handful of the most important paths (or even none at all) which allows the pages to be Server Rendered when it gets traffic, then Statically rendered for subsequent visitors.

If you have very important pages, or very high traffic pages, you could pass them as paths, but I imagine most sites don't need every single page to be statically generated at build time, and can instead be statically generated if/when they get traffic.

This will help reduce load on the WordPress server and will significantly increase build times (which will reduce cost of tools that charge for build time, etc).

As a lot of users in the community have been using this starter as a starting point, it would be nice to "fix" this issue here and encourage folks to use ISR as default, and pass paths only when they really feel like they need to.

The issue starts here: https://github.com/WebDevStudios/nextjs-wordpress-starter/blob/1ab955633110c8c1cbe2db0a6f02740b26b23317/functions/wordpress/postTypes/getPostTypeStaticPaths.js#L14-L85

This function is fetching 10,000 nodes from WPGraphQL. WPGraphQL prevents more than 100 per request by default, so either this will only be returning 100 items, or modifications need to happen in WPGraphQL to support this use case, and I would generally advise against this.

Once the 10,000 nodes are returned, they're passed as paths and then Next proceeds to build every single one of those 10,000 pages (or however many there are), and for each page that means another GraphQL query (or more) are being sent to the WordPress server, overwhelming the server.

If we change this to something similar to:

export async function getStaticPaths() {
    return {
        paths: [],
        fallback: 'blocking'
    }
}

Then no pages will be built at build time, instead they will be Server Rendered on initial visit, and Static for the next visitors.

If you have important pages, you could put them in the paths:

export async function getStaticPaths() {
    return {
        paths: [
          '/some-breaking-news-story',
        ],
        fallback: 'blocking'
    }
}

But even for those cases, I imagine it would be common for content creators to want those pages to be available close to the time that the content is published in WordPress, and not after waiting for builds to complete, so ISR is still probably going to be the best option, even for these cases where you have very important pages you want to make sure are always static.

I believe it would be a great benefit to the community to remove the fetching of 10,000 nodes from this example to reduce the changes of users running into issues with this.

I'd be happy to hop on a Zoom or anything to discuss further if it's helpful.

Thanks!

πŸ™πŸ»

jasonbahl commented 2 years ago

FWIW, you can reference this codebase to see how I'm doing stuff for nextjs.wpgraphql.com, if it's helpful: https://github.com/jasonbahl/headless-wp-template-hierarchy