framework icon indicating copy to clipboard operation
framework copied to clipboard

fix(nuxt): only fetch payloads for prerendered routes

Open danielroe opened this issue 2 years ago • 7 comments

🔗 Linked issue

https://github.com/nuxt/framework/issues/6411

resolves https://github.com/nuxt/framework/issues/7769

❓ Type of change

  • [ ] 📖 Documentation (updates to the documentation or readme)
  • [x] 🐞 Bug fix (a non-breaking change that fixes an issue)
  • [x] 👌 Enhancement (improving an existing functionality like performance)
  • [ ] ✨ New feature (a non-breaking change that adds functionality)
  • [ ] ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

This PR adds support for an app manifest (at /manifest.json) which contains, at the moment, simply a list of routes for which there are corresponding payloads. With the implementation of route rules this manifest can be extended/modified. In addition, it can be extended with a build hash to allow version change detection.

With this PR:

  • we will not fetch payloads for pages that were not prerendered (they would give a 404)
  • will will fetch all payloads if there is an SSR/hybrid app
  • we can fetch payloads in development mode if experimental.payloadExtraction is set by the user to true (there are more edge cases, such as CORS, with client-side fetching, so I felt it was better to set default to false in dev mode so user handles them properly, in case they are not prerendering all their pages).
  • support for hybrid payload extraction (e.g. some but not all routes have SSR payloads) is deferred to implementation of route rules

(Note, @pi0, I'm also happy to go ahead and implement route rules, but thought we could adopt this approach for now and modify it at that point.)

📝 Checklist

  • [x] I have linked an issue or discussion.
  • [ ] I have updated the documentation accordingly.

danielroe avatar Sep 24 '22 11:09 danielroe

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

codesandbox[bot] avatar Sep 24 '22 11:09 codesandbox[bot]

Deploy Preview for nuxt3-docs canceled.

Name Link
Latest commit 109922edae0a754178424ed71ec60a15e2b36fd5
Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/632f5ce7154ff900089adb84

netlify[bot] avatar Sep 24 '22 11:09 netlify[bot]

@pi0 How can I help on this? What's the reason this is marked as draft/pending?

danielroe avatar Sep 27 '22 15:09 danielroe

🙏

ice-open-source avatar Oct 16 '22 19:10 ice-open-source

@pi0 Would you let me know why you have marked this as a draft/pending? There are some changes that I would like to make to update this to use the route rules that were implemented after this was opened, but I'm currently waiting to hear back from you on this.

danielroe avatar Nov 10 '22 13:11 danielroe

Having a /manifest.json is a nice idea to detect a new deployment!

But I'm against adding every prerendered route to a single manifest file, is not good for performance and not scalable for any real-world project with dozens of routes. (instead we should keep route rules that predict prerendering possibility and check them with radix3 utils)

  • Websites build with nuxt build and deployed to server, are able to render payloads in runtime. We only need to know if route is configured to prerendered or not (via route rules)
  • Websites build with nuxt generate, are considered full-static. In addition to manifest.json that has basic build info such as prerender time, by trying to fetch payload paths, we basically check if they are (or can be) pre-rendered or not in the background without maintaining a centralized manifest. We can cache 404 results to avoid trying them in same session (or until next deployment) and avoid spamming console.

pi0 avatar Nov 10 '22 14:11 pi0

On board with nuxt build 👍

So, just to understand the situation with :full static mode - you're saying that from your point of view, 404s are an inevitable evil, with two possible mitigations: We can (1) prevent fetching them if there is a prerender route rule, and (2) prevent retrying a payload url that has already 404ed.

danielroe avatar Nov 10 '22 15:11 danielroe

Are there any updates on if/when this, or a solution to solve this problem will be completed?

Currently having this issue in production where payloads for non prerendered routes are being requested resulting in a 404 which is not ideal.

ConnorCairns avatar Dec 19 '22 15:12 ConnorCairns

~> https://github.com/nuxt/nuxt/pull/18419

danielroe avatar Jan 21 '23 14:01 danielroe