Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

fix(nuxt): only fetch payloads for prerendered routes #7805

Closed
wants to merge 2 commits into from

Conversation

danielroe
Copy link
Member

πŸ”— Linked issue

nuxt/nuxt#14507

resolves nuxt/nuxt#15024

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ 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

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

@danielroe danielroe added bug Something isn't working enhancement New feature or request πŸ”¨ p3-minor-bug Priority 3: a bug in an edge case that only affects very specific usage labels Sep 24, 2022
@danielroe danielroe requested a review from pi0 September 24, 2022 11:33
@danielroe danielroe self-assigned this Sep 24, 2022
@codesandbox
Copy link

codesandbox bot commented Sep 24, 2022

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

@netlify
Copy link

netlify bot commented Sep 24, 2022

βœ… Deploy Preview for nuxt3-docs canceled.

Name Link
πŸ”¨ Latest commit 109922e
πŸ” Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/632f5ce7154ff900089adb84

@pi0 pi0 added the pending label Sep 26, 2022
@pi0 pi0 marked this pull request as draft September 26, 2022 09:31
@danielroe
Copy link
Member Author

danielroe commented Sep 27, 2022

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

@ice-open-source
Copy link

πŸ™

@danielroe
Copy link
Member Author

danielroe commented Nov 10, 2022

@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.

@pi0
Copy link
Member

pi0 commented Nov 10, 2022

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.

Copy link
Member Author

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.

@ConnorCairns
Copy link

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.

@danielroe
Copy link
Member Author

~> nuxt/nuxt#18419

@danielroe danielroe closed this Jan 21, 2023
@danielroe danielroe deleted the feat/route-manifest branch January 21, 2023 14:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3.x bug Something isn't working enhancement New feature or request πŸ”¨ p3-minor-bug Priority 3: a bug in an edge case that only affects very specific usage pending
Projects
None yet
4 participants