-
-
Notifications
You must be signed in to change notification settings - Fork 233
fix: provide payload fallback json #1186
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis pull request adds JSON fallback support to ISR. modules/isr-fallback.ts defines Possibly related PRs
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
nuxt.config.ts
Outdated
| interface ISRConfigOptions { | ||
| fallback?: 'html' | 'json' | ||
| } | ||
| function getISRConfig(expirationSeconds: number, options: ISRConfigOptions = {}) { | ||
| if (options.fallback) { | ||
| return { | ||
| expiration: expirationSeconds, | ||
| fallback: 'spa.prerender-fallback.html', | ||
| fallback: | ||
| options.fallback === 'html' ? 'spa.prerender-fallback.html' : 'payload-fallback.json', | ||
| } as { expiration: number } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, find the nuxt.config.ts file
find . -name "nuxt.config.ts" -type fRepository: npmx-dev/npmx.dev
Length of output: 99
🏁 Script executed:
# Check the file size first
wc -l ./nuxt.config.ts
# Then read lines 280-305 to get full context
sed -n '280,305p' ./nuxt.config.tsRepository: npmx-dev/npmx.dev
Length of output: 604
🏁 Script executed:
# Check if getISRConfig is called elsewhere
rg "getISRConfig" ./nuxt.config.ts
# Also check for any imports or exports
rg "getISRConfig" . --type ts --type tsxRepository: npmx-dev/npmx.dev
Length of output: 927
🏁 Script executed:
# Check the TypeScript strict settings to confirm strict type-safety is enforced
cat ./tsconfig.json | grep -A 20 '"strict"'Repository: npmx-dev/npmx.dev
Length of output: 43
🏁 Script executed:
# Check if tsconfig.json exists
ls -la tsconfig.json 2>&1 || echo "File not found"
# Try reading the file
cat tsconfig.json 2>&1 | head -50Repository: npmx-dev/npmx.dev
Length of output: 442
Remove the type assertion to restore type safety.
The as { expiration: number } cast on line 296 hides the fallback field from the return type, even though the function includes it in the returned object when options.fallback is provided. This violates strict type-safety requirements. Define an explicit return type that includes an optional fallback property and remove the assertion.
🔧 Proposed fix
interface ISRConfigOptions {
fallback?: 'html' | 'json'
}
+interface ISRConfig {
+ expiration: number
+ fallback?: 'spa.prerender-fallback.html' | 'payload-fallback.json'
+}
-function getISRConfig(expirationSeconds: number, options: ISRConfigOptions = {}) {
+function getISRConfig(expirationSeconds: number, options: ISRConfigOptions = {}): ISRConfig {
if (options.fallback) {
return {
expiration: expirationSeconds,
fallback:
options.fallback === 'html' ? 'spa.prerender-fallback.html' : 'payload-fallback.json',
- } as { expiration: number }
+ }
}
return {
expiration: expirationSeconds,
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| interface ISRConfigOptions { | |
| fallback?: 'html' | 'json' | |
| } | |
| function getISRConfig(expirationSeconds: number, options: ISRConfigOptions = {}) { | |
| if (options.fallback) { | |
| return { | |
| expiration: expirationSeconds, | |
| fallback: 'spa.prerender-fallback.html', | |
| fallback: | |
| options.fallback === 'html' ? 'spa.prerender-fallback.html' : 'payload-fallback.json', | |
| } as { expiration: number } | |
| interface ISRConfigOptions { | |
| fallback?: 'html' | 'json' | |
| } | |
| interface ISRConfig { | |
| expiration: number | |
| fallback?: 'spa.prerender-fallback.html' | 'payload-fallback.json' | |
| } | |
| function getISRConfig(expirationSeconds: number, options: ISRConfigOptions = {}): ISRConfig { | |
| if (options.fallback) { | |
| return { | |
| expiration: expirationSeconds, | |
| fallback: | |
| options.fallback === 'html' ? 'spa.prerender-fallback.html' : 'payload-fallback.json', | |
| } | |
| } | |
| return { | |
| expiration: expirationSeconds, | |
| } | |
| } |
|
For my own nuxt edification, would you mind explaining what this does and what this PR is solving for? 🙏🏼 |
|
Deployment failed with the following error: Learn More: https://vercel.link/invalid-route-source-pattern |
|
@serhalp this provides fallback json files to speed up initial fetching of payloads when the cache is cold. it's not a nuxt thing in particular, and we do need to solve by speeding up the rendering of payloads for package pages, instead. |
No description provided.