-
-
Notifications
You must be signed in to change notification settings - Fork 729
fix(module): work with nitro cache by registering routes before nitro… #3617
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Deploying content with
|
| Latest commit: |
fa6efdd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a38d9af4.content-f0q.pages.dev |
| Branch Preview URL: | https://fix-nitro-cache.content-f0q.pages.dev |
| // Due to prerender enabling in the module, Nuxt create a route for each collection | ||
| // These routes cause issue while enabling cache in Nuxt. | ||
| // So we need to add a server handler for each collection to handle the request. | ||
| manifest.collections.map(async (collection) => { |
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.
Using .map(async ...) without awaiting creates unnecessary promises that are discarded, and should use .forEach() instead for synchronous operations.
View Details
📝 Patch Details
diff --git a/src/presets/cloudflare.ts b/src/presets/cloudflare.ts
index 501b30ba..ad0b4bf7 100644
--- a/src/presets/cloudflare.ts
+++ b/src/presets/cloudflare.ts
@@ -7,7 +7,7 @@ import { collectionDumpTemplate } from '../utils/templates'
export default definePreset({
name: 'cloudflare',
setup(_options, _nuxt, { resolver, manifest }) {
- manifest.collections.map(async (collection) => {
+ manifest.collections.forEach((collection) => {
addServerHandler({
route: `/__nuxt_content/${collection.name}/sql_dump.txt`,
handler: resolver.resolve('./runtime/presets/cloudflare/database-handler'),
@@ -25,7 +25,7 @@ export default definePreset({
nitroConfig.handlers ||= []
// Add raw content dump
- manifest.collections.map(async (collection) => {
+ manifest.collections.forEach((collection) => {
if (!collection.private) {
addTemplate(collectionDumpTemplate(collection.name, manifest))
}
diff --git a/src/presets/node.ts b/src/presets/node.ts
index 4b4bb340..f805dc24 100644
--- a/src/presets/node.ts
+++ b/src/presets/node.ts
@@ -8,7 +8,7 @@ export default definePreset({
// Due to prerender enabling in the module, Nuxt create a route for each collection
// These routes cause issue while enabling cache in Nuxt.
// So we need to add a server handler for each collection to handle the request.
- manifest.collections.map(async (collection) => {
+ manifest.collections.forEach((collection) => {
addServerHandler({
route: `/__nuxt_content/${collection.name}/sql_dump.txt`,
handler: resolver.resolve('./runtime/presets/node/database-handler'),
Analysis
Unnecessary async callbacks in preset collection loops create discarded promises
What fails: src/presets/node.ts (line 11) and src/presets/cloudflare.ts (lines 10, 28) use .map(async (collection) => { addServerHandler(...) }) without awaiting the returned promise array, creating unnecessary promise objects that are immediately garbage collected.
How to reproduce: The code works functionally but creates and discards promises:
// Current pattern (creates unused promises)
const result = manifest.collections.map(async (collection) => {
addServerHandler({ route: `...` })
})
// result is Array<Promise<undefined>> - these promises are never resolved/awaitedResult: Arrays of unused Promise objects are created and garbage collected, with no functional benefit. The async keyword is semantically incorrect since addServerHandler() is synchronous (returns void).
Expected behavior: Use .forEach() for side effects without return values, matching the pattern in src/utils/templates.ts line 52 which correctly uses await Promise.all() when async operations need awaiting, and the established codebase pattern where synchronous calls use .forEach().
Reference: Per Nuxt Kit documentation, addServerHandler() signature is function addServerHandler (handler: NitroEventHandler): void - synchronous with no async behavior.
Fix: Replace .map(async (collection) => {...}) with .forEach((collection) => {...}) to eliminate unnecessary promise creation and clarify intent.
|
|
||
| // Add raw content dump to public assets | ||
| nitroConfig.publicAssets.push({ dir: join(nitroConfig.buildDir!, 'content', 'raw'), maxAge: 60 }) | ||
| nitroConfig.handlers.push({ |
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.
Using .map(async ...) without awaiting creates unnecessary promises that are discarded, and should use .forEach() instead for synchronous operations.
View Details
📝 Patch Details
diff --git a/src/presets/cloudflare.ts b/src/presets/cloudflare.ts
index 501b30ba..ad0b4bf7 100644
--- a/src/presets/cloudflare.ts
+++ b/src/presets/cloudflare.ts
@@ -7,7 +7,7 @@ import { collectionDumpTemplate } from '../utils/templates'
export default definePreset({
name: 'cloudflare',
setup(_options, _nuxt, { resolver, manifest }) {
- manifest.collections.map(async (collection) => {
+ manifest.collections.forEach((collection) => {
addServerHandler({
route: `/__nuxt_content/${collection.name}/sql_dump.txt`,
handler: resolver.resolve('./runtime/presets/cloudflare/database-handler'),
@@ -25,7 +25,7 @@ export default definePreset({
nitroConfig.handlers ||= []
// Add raw content dump
- manifest.collections.map(async (collection) => {
+ manifest.collections.forEach((collection) => {
if (!collection.private) {
addTemplate(collectionDumpTemplate(collection.name, manifest))
}
diff --git a/src/presets/node.ts b/src/presets/node.ts
index 4b4bb340..f805dc24 100644
--- a/src/presets/node.ts
+++ b/src/presets/node.ts
@@ -8,7 +8,7 @@ export default definePreset({
// Due to prerender enabling in the module, Nuxt create a route for each collection
// These routes cause issue while enabling cache in Nuxt.
// So we need to add a server handler for each collection to handle the request.
- manifest.collections.map(async (collection) => {
+ manifest.collections.forEach((collection) => {
addServerHandler({
route: `/__nuxt_content/${collection.name}/sql_dump.txt`,
handler: resolver.resolve('./runtime/presets/node/database-handler'),
Analysis
Unhandled promise rejections from unnecessary async callbacks in preset setup methods
What fails: setupNitro() in src/presets/cloudflare.ts (lines 10, 28) and src/presets/node.ts (line 11) use .map(async ...) without awaiting the returned promises. Since the callbacks (addServerHandler and addTemplate) are synchronous, this pattern creates unnecessary Promise objects and can result in unhandled promise rejections if any error occurs inside the async callback.
How to reproduce:
// In src/presets/cloudflare.ts setupNitro method, if any error occurs:
manifest.collections.map(async (collection) => {
if (!collection.private) {
addTemplate(collectionDumpTemplate(collection.name, manifest))
// If an error is thrown here, it becomes an unhandled promise rejection
// because the returned promises are never awaited
}
})Result: Any errors thrown inside the async callback become unhandled promise rejections. These are silently lost unless there's a global unhandled rejection handler. The code still functions for the synchronous operations, but error handling is broken.
Expected: Since addServerHandler() and addTemplate() from @nuxt/kit are synchronous functions, the async wrapper is unnecessary and creates unhandled rejection scenarios. Using .forEach() eliminates the promise wrapping and ensures errors propagate normally.
Fix applied: Changed .map(async (collection) => {...}) to .forEach((collection) => {...}) in:
- src/presets/cloudflare.ts line 10 (setup method)
- src/presets/cloudflare.ts line 28 (setupNitro method)
- src/presets/node.ts line 11 (setup method)
… config hook
🔗 Linked issue
resolves #3613
❓ Type of change
📚 Description
This was an attempt to solve #3613. But it seems that it works only if we move route to
/api/**.Nitro cache + server handler on
modules:donehook does not work if it is not prefixed with/api/cc @pi0 @atinux📝 Checklist