search

vercel / next.js

The React Framework
https://nextjs.org
MIT License
127.23k stars 27.02k forks source link

Preview mode doesn't work for pages not specified by getStaticPaths #16681

Closed benyap closed 3 years ago

benyap commented 4 years ago

Bug report

Describe the bug

Trying to access dynamic/catch-all routes in preview mode that are not specified by getStaticPaths returns 404 Not Found in production. I have verified that preview mode cookies are being sent on the requests, so that's not the issue.

In development, the behaviour works as expected, though this might be due to the fact that getStaticProps is called on every request when in development mode.

To Reproduce

Repository: https://github.com/nextjs-preview-issue/nextjs-preview-issue

Demo: https://nextjs-preview-issue-demo.vercel.app

Enable preview mode by clicking the "PREVIEW MODE" button and enter token1 or token2 into the prompt.

See "Additional context" for notable code snippets from the demo repository.

Expected behaviour

From the blog post:

Preview Mode allows users to bypass the statically generated page to on-demand render (SSR) a draft page from for example a CMS.

When trying to access pages that match a dynamic or catch-all route, I would expect that it always renders the page when in preview mode, even if it is not specified by getStaticPaths, because preview mode is intended for on-demand render of draft pages.

I considered using fallback: true in getStaticPaths, but that slightly changes behaviour in production regardless of preview mode - I shouldn't need the server to check and render pages when not in preview mode.

Running the demo on a local machine in development mode using yarn dev exhibits the behaviour I expected.

System information

Additional context

Notable code snippets/files

Frontend: src/components/Preview.tsx

This function is attached to a button's onClick handler which enables preview mode.

const previewMode = () => {
  const token = prompt("Enter preview token.");
  if (token) push(`/api/preview?token=${token}`)
};

Backend: src/pages/api/preview.ts

This API route enables preview mode and redirects the user to a page that matches the specified pageId if given, otherwise it redirects them to the index page. In the demo, the pageId parameter is not used (see above) so the user will always redirected to the index page when enabling preview mode.

Backend: src/utils.ts

This file contains the getStaticPaths and getStaticProps implementations used by the dynamic route (blog/[id].tsx) and the catch all route ([...slug].tsx).

The getStaticPaths implementations only lists pages/posts that have published: true because I do not want draft pages rendered at build time. getStaticProps retrieves the page's/post's data using the slug in the route params regardless of preview mode or not.

When in preview mode, getStaticProps passes the preview data to the page as the preview prop. The page will check this prop and render "(preview)" in the title when this is a non-null value.

See: Index page Blog post page Catch-all page

Backend: src/data.ts

This file contains the mock data used in the demo. In a real project these values would be retrieved from a database.

My use case and how I came across this issue

I'm building a page builder interface in my project and I want to allow users to preview pages they've created in the CMS that have not been published yet. Pages in my CMS database have a publishState flag. My getStaticPaths function only lists pages that have publishState set to "published" as I do not want draft pages to be rendered at build time. When draft pages are created, they have a publishState of "draft".

I deploy my project to Vercel and allow the users to publish new content by using a deploy hook to rebuild the project. Before the deploy hook is called, pages that are to be published have the state set to "published" so that getStaticPaths returns their ids for rendering during build time.

Timer commented 4 years ago

There's two ways to solve this:

I'm not sure which is the most optimal fix, but we'll think about this. Thanks for the issue!

benyap commented 4 years ago

There's two ways to solve this:

  • Development should also 404 in preview mode for a fallback: false page and an unmatched slug
  • Production should match a path not returned from paths: [] even if fallback: false, making preview mode work like fallback: true

I'm not sure which is the most optimal fix, but we'll think about this. Thanks for the issue!

I personally think the second option is more flexible and gives more control to what can be done in preview mode, so my vote would be for that one haha

xiata commented 4 years ago

Somewhat related, on a /[...slug].js route I have, getServerSideProps({ params, preview }), preview is filled in on development builds, but is always undefined in production builds, despite __prerender_bypass and __next_preview_data cookies being present.

ericclemmons commented 4 years ago

I just ran into this issue myself and was a bit surprised that it didn't work.

I do believe that development should match production behavior, which means (today) it should 404.

My expectation was that fallback: true and preview mode would both behave about the same: deferring to the server like getServerSideProps for new paths.

josias-r commented 4 years ago

If getStaticPaths would receive a context parameter similar to getStaticProps including the preview boolean, you could control the behaviour yourself as a dev, whether fallback should be enabled in preview mode or not. 🤷🏻‍♂️

joshuabaker commented 3 years ago

10995

tremby commented 3 years ago

I was auditing my app just now to make sure previewData is always passed through to API queries, and was surprised to find that previewData is not part of GetStaticPathsContext. What's the reason for that? If it were present, I'd be able to tell Next the correct list of paths for a given preview, no? Then there wouldn't be any need for fallback:true or other special behaviour in my use case.

vajajak commented 3 years ago

I've also stumbled upon this issue today, which I have never noticed earlier. The reason behind that is because I was only testing this locally (either with npm run dev or npm run build && npm run start). This comment also reassured me that I'm doing everything right and that there's no need to have a preview mode aware getStaticPaths, since they're not designed to run at runtime and Next.js is treating every request as if it had fallback: true set. As far as i know, this problem is unique to the Vercel environment as that's the only place where I was able to reproduce this issue. @Timer perhaps this might be more of a Vercel problem?

ambrauer commented 3 years ago

I can confirm @vajajak's findings, which we just stumbled on today as well. This issue only appears to surface when deployed on Vercel. Running locally in production mode (with next build && next start), preview mode is working as expected (calling getStaticProps for all requests regardless of presence in getStaticPaths) with fallback: false.

alexburner commented 3 years ago

We're having this same experience, we're using getStaticPaths and can't preview pages without publishing them and running a site build. We're also hosting on Vercel.

The preview docs make it sound like this specific use case should work:

In the Pages documentation and the Data Fetching documentation, we talked about how to pre-render a page at build time (Static Generation) using getStaticProps and getStaticPaths.

Static Generation is useful when your pages fetch data from a headless CMS. However, it’s not ideal when you’re writing a draft on your headless CMS and want to preview the draft immediately on your page. You’d want Next.js to render these pages at request time instead of build time and fetch the draft content instead of the published content. You’d want Next.js to bypass Static Generation only for this specific case.

Next.js has a feature called Preview Mode which solves this problem. Here are instructions on how to use it.

https://nextjs.org/docs/advanced-features/preview-mode

Reading between the lines, it seems like this is indicating it should behave like @Timer's second option?

  • Production should match a path not returned from paths: [] even if fallback: false, making preview mode work like fallback: true

And that a page could be previewed, even if it's unpublished (its path isn't returned by getStaticPages) and no site build has been run since creating the page draft.

alexburner commented 3 years ago

Update: I was able to enable the desired preview behavior, with only two small changes!

In this case, for a [slug].tsx page:

  1. Change fallback: false to fallback: 'blocking' in getStaticPaths()
  2. Return { notFound: true } from getStaticProps() if querying with context.params.slug fails to find data

Since our site is small, we're doing full SSG instead of ISR, and this code has no impact on the normal production site.

vajajak commented 3 years ago

Update: I was able to enable the desired preview behavior, with only two small changes!

In this case, for a [slug].tsx page:

  1. Change fallback: false to fallback: 'blocking' in getStaticPaths()
  2. Return { notFound: true } from getStaticProps() if querying with context.params.slug fails to find data

Since our site is small, we're doing full SSG instead of ISR, and this code has no impact on the normal production site.

@alexburner This might be a useful workaround for now but is certainly not ideal. When you quit the preview mode, the page stays generated (more specifically - the first generated version - if you're not also using ISR) and can be accessed publicly with a url, even though it's still a draft, essentially defeating the purpose of Preview Mode.

joshuabaker commented 3 years ago

@vajajak That’s a valid point. Thinking out loud here, would passing revalidate in combo with @alexburner’s approach sort that?

vajajak commented 3 years ago

@joshuabaker Yes and No. Let's see what happens when we enable revalidate (for testing purposes let's say 1 second).

alexburner commented 3 years ago

When you quit the preview mode, the page stays generated (more specifically - the first generated version - if you're not also using ISR) and can be accessed publicly with a url, even though it's still a draft, essentially defeating the purpose of Preview Mode.

@vajajak I hadn't considered this — not an ideal situation

joshuabaker commented 3 years ago

@Timer This might be considered a serious concern for organisations that aren’t legally allowed to publish information ahead of a date (e.g. earnings reports). Is there an option available for us not mentioned above?

ijjk commented 3 years ago

Hi, this has been updated, please give it a try by re-deploying your application!

Note: with fallback: false pages and previewing a non-prerendered path a 404 status will be shown on Vercel to prevent crawlers indexing the 404 page although this will be updated in the future to only have the 404 status when not in preview mode.

vajajak commented 3 years ago

@ijjk Thanks a lot for the fix. Seems like it's working fine now 👍

ijjk commented 3 years ago

Closing as this should be working correctly now per above

nickwesselman commented 3 years ago

Can someone explain what the fix was / how we are supposed to take advantage? From our POV, the scenario is:

While this scenario works locally, Vercel still returns a 404 when the page is requested, even with preview enabled. This effectively means that enabling fallback is a requirement for our Next.js SDK.

Is there a workaround or an additional fix planned for this @ijjk?

ijjk commented 3 years ago

Hi, can you provide a repo with a minimal reproduction where you are seeing this not working? As noted in the above replies it is working correctly for users.

nickwesselman commented 3 years ago

Hey @ijjk -- happy to create this, but to be clear, you describe the problematic behavior above already:

this will be updated in the future to only have the 404 status when not in preview mode

Is there a timeframe for this? Thanks!

ijjk commented 3 years ago

Ah so the problem is with the status code only or are you seeing the 404 page rendered as well? That note was specifically meaning the status code will be a 404 in preview mode for fallback: false pages but the preview content should render correctly.

nickwesselman commented 3 years ago

@ijjk Thanks, our issue may be that we just need to gracefully handle the 404, we are going to investigate further.

nickwesselman commented 3 years ago

@ijjk To close the loop, indeed we just needed to gracefully handle the 404 response when fetching the preview. Thanks!

balazsorban44 commented 2 years ago

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.