-
Notifications
You must be signed in to change notification settings - Fork 95
fix: detect static nitro presets #760
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
|
@onmax is attempting to deploy a commit to the NuxtLabs Team on Vercel. A member of the Team first needs to authorize it. |
commit: |
5203a0f to
23e38fd
Compare
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.
I don't think blocking static presets will fix the issue 🤔
NuxtHub docs already mention pre-render all pages by enabling prerender.crawLinks: true, but adding this manually still results in prerender errors for sql_dump.txt
AFAIK, the Nitro static presets should be same as what the docs describe, since they also enable prerender.crawLinks https://github.com/nitrojs/nitro/blob/6e801e223c498ab3888434bc9704a576ce3c565c/src/presets/_static/preset.ts#L13
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.
Thanks for the review!
I think there's a misunderstanding here. Static presets are NOT equivalent to prerender.crawlLinks: true:
| Static preset | Server + prerender | |
|---|---|---|
| Server deployed | ❌ No _worker.js |
✅ Yes |
| D1/KV/R2 bindings | ❌ No runtime | ✅ Work at runtime |
| API routes | ❌ 404 | ✅ Work |
NuxtHub features require Cloudflare bindings that only exist at runtime. Static presets don't deploy a server, so there's no runtime.
From Nitro source:
cloudflare-pages-staticextendsstaticwithstatic: true-> skips server build entirelycloudflare-pagesextendsbase-worker-> builds_worker.js
The correct approach for pre-rendered pages with NuxtHub:
- Use
cloudflare-pages(with server) - Set
nitro.prerender.crawlLinks: true
That said, we could make this smarter: only block when hub features (db/kv/blob/cache) are actually enabled. If none are enabled, static could be allowed since NuxtHub is a no-op. This is a question for maintainers: do we want to implement that?
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.
Thanks for the quick response! 😊
I tried the suggested setup (cloudflare-pages + prerender.crawlLinks: true) and it still fails to prerender sql_dump.txt.
So, I think this doesn't seem specific to static presets or missing runtime. Like, other pages prerender fine, only sql_dump.txt fails.
I've updated the repro to use the pre-render all pages approach, so it's reproducible with npm run build:
https://stackblitz.com/edit/github-n1pcahgr?file=nuxt.config.ts,package.json
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.
Thanks for the detailed feedback!The issue isn't specific to static presets.
Root cause: When crawlLinks: true, Nitro discovers /__nuxt_content/:collection/sql_dump.txt routes during prerendering and tries to prerender them. The handler fails because it needs cloudflare ASSETS binding that's only available at runtime.
I've submitted a fix upstream in @nuxt/content:
- Issue: sql_dump.txt routes fail prerender with crawlLinks: true nuxt/content#3667
- PR: fix: exclude sql_dump.txt routes from prerendering nuxt/content#3668
Workaround until the fix is released:
// nuxt.config.ts
nitro: {
prerender: {
ignore: [/^\/__nuxt_content\/.*\/sql_dump\.txt/]
}
}
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.
Thanks for tracking down the upstream issue and sharing the workaround! Ignoring the route during prerendering seems reasonable for now.
That said, I'm not convinced the upstream fix should unconditionally exclude sql_dump.txt from prerendering, given the changes applied to the node and cloudflare presets. Prerendering sql_dump.txt should be useful because it prevents these assets from being resolved at runtime.
I tested the workaround on my side with @nuxthub/core 0.10.4 and @nuxt/content 3.10.0, and it works at both build time and runtime:
| Preset | Workaround worked |
|---|---|
| default (crawlLinks-based, as mentioned in NuxtHub docs) | ✅ |
cloudflare-pages + crawlLinks |
✅ |
static |
✅ |
cloudflare-pages-static |
✅ |
For reference, these version combinations might still be useful for further investigation as noted in the original issue. These results were observed with crawlLinks-based prerendering enabled or when using static presets, and before applying the workaround:
| @nuxthub/core | @nuxt/content | sql_dump.txt prerendered |
|---|---|---|
| not used | 3.10.0 | ✅ |
| 0.10.4 | 3.10.0 | ❌ |
| 0.10.3 | 3.9.0 | ✅ |
| 0.10.4 | 3.9.0 | ✅ |
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.
thanks! I will close this PR
tracking issue now at PR: nuxt/content#3668
4a53a71 to
23e38fd
Compare
Fixes #754
nitro.preset: 'static'bypasses NuxtHub's static check. Build proceeds instead of failing with clear error.Reproduce bug
Verify fix