fix(runtime): use dynamic import for #content/adapter to prevent prerender failure - #3830
Open
gepotumu wants to merge 2 commits into
Open
fix(runtime): use dynamic import for #content/adapter to prevent prerender failure#3830gepotumu wants to merge 2 commits into
#content/adapter to prevent prerender failure#3830gepotumu wants to merge 2 commits into
Conversation
…erender failure When `sqliteConnector: 'bun'` is configured and the build runs on Node.js, the prerender stage fails because Node.js cannot resolve the `bun:` protocol. Root cause: `database.server.ts` used a static top-level import for `#content/adapter`. Node.js ESM loader resolves all static imports at module load time, regardless of whether the binding is called at runtime. During prerender only `localAdapter` is used, but the static import of `adapter` still forces resolution of `bun:sqlite`. Fix: Replace the static import with a lazy dynamic `import()` that is only resolved in the production code path (non-prerender, non-dev). This makes `loadDatabaseAdapter` async, which is a minimal API change since all callers already operate in async contexts. Closes nuxt#3829 Co-authored-by: Cursor <cursoragent@cursor.com>
|
Someone is attempting to deploy a commit to the Nuxt Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe runtime now loads the production database adapter through a cached dynamic import. Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/runtime/internal/database.server.ts`:
- Around line 24-30: Update the connector initialization flow around the
module-level db guard and getAdapter so concurrent first calls share a cached
initialization promise, ensuring the adapter factory runs only once and all
callers receive the same connector. Preserve the existing dev/localAdapter and
production adapter selection behavior, and add a Promise.all regression test
covering concurrent initial calls.
In `@test/mock/content-adapter.ts`:
- Line 3: Rename the unused prepare callback parameter from sql to _sql in
test/mock/content-adapter.ts:3-3, test/mock/content-local-adapter.ts:3-3, and
each affected callback in test/unit/database.server.prerender.test.ts:48-48,
57-57, and 87-87, while preserving the mock interface and callback behavior.
In `@test/unit/database.server.prerender.test.ts`:
- Line 75: Replace the `config as any` casts in
`test/unit/database.server.prerender.test.ts` at lines 75, 109, and 113 with one
shared fixture typed as `RuntimeConfig['content']`, and pass that fixture
directly to `loadDatabaseAdapter` at each site.
- Around line 31-42: The static import assertion using staticImportPattern must
reject every top-level `#content/adapter` import form, including named, namespace,
side-effect, and combined imports, while continuing to allow dynamic
import('`#content/adapter`'). Replace the regex-only check with an import parser
or parser-backed assertion that distinguishes static imports from dynamic
imports.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 21366656-2258-4171-b08b-41890e02e463
📒 Files selected for processing (7)
src/runtime/api/query.post.tssrc/runtime/internal/database.server.tstest/mock/content-adapter.tstest/mock/content-local-adapter.tstest/mock/content-manifest.tstest/unit/database.server.prerender.test.tsvitest.config.ts
Comment on lines
24
to
30
| if (!db) { | ||
| if (import.meta.dev || ['nitro-prerender', 'nitro-dev'].includes(import.meta.preset as string)) { | ||
| db = localAdapter(refineDatabaseConfig(localDatabase)) | ||
| } | ||
| else { | ||
| const adapter = await getAdapter() | ||
| db = adapter(refineDatabaseConfig(database)) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make connector initialization atomic.
Concurrent production requests can both pass if (!db) before either resumes from await getAdapter(). Both requests then call the adapter factory. One connector is discarded when the later assignment overwrites db.
Cache the connector initialization promise, not only the module import. Add a Promise.all regression test for concurrent first calls.
Proposed fix
let db: Connector
let _adapterPromise: Promise<(opts: unknown) => Connector> | undefined
+let _databasePromise: Promise<Connector> | undefined
export default async function loadDatabaseAdapter(config: RuntimeConfig['content']) {
const { database, localDatabase } = config
if (!db) {
- if (import.meta.dev || ['nitro-prerender', 'nitro-dev'].includes(import.meta.preset as string)) {
- db = localAdapter(refineDatabaseConfig(localDatabase))
- }
- else {
- const adapter = await getAdapter()
- db = adapter(refineDatabaseConfig(database))
+ if (!_databasePromise) {
+ _databasePromise = (async () => {
+ if (import.meta.dev || ['nitro-prerender', 'nitro-dev'].includes(import.meta.preset as string)) {
+ return localAdapter(refineDatabaseConfig(localDatabase))
+ }
+ const adapter = await getAdapter()
+ return adapter(refineDatabaseConfig(database))
+ })()
}
+ db = await _databasePromise
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/runtime/internal/database.server.ts` around lines 24 - 30, Update the
connector initialization flow around the module-level db guard and getAdapter so
concurrent first calls share a cached initialization promise, ensuring the
adapter factory runs only once and all callers receive the same connector.
Preserve the existing dev/localAdapter and production adapter selection
behavior, and add a Promise.all regression test covering concurrent initial
calls.
- Prefix unused parameters with `_` to satisfy @typescript-eslint/no-unused-vars - Replace `as any` casts with a shared typed config fixture - Remove unused variable assignment Co-authored-by: Cursor <cursoragent@cursor.com>
commit: |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3829
When
sqliteConnector: 'bun'is configured and the build runs on Node.js (e.g.nuxt build --preset bun), the prerender stage fails with:Root Cause
database.server.tsuses a static top-level import for#content/adapter:Node.js ESM loader resolves ALL static imports at module load time, regardless of whether the imported binding is actually called. During prerender, only
localAdapteris used (line 18), but the static import forces Node.js to resolvebun:sqlite— which fails becausebun:is not a valid Node.js URL scheme.Fix
Replace the static import with a lazy dynamic
import()that is only resolved in the production code path:This makes
loadDatabaseAdapterasync — a minimal API change since all callers (query.post.tsevent handler and_checkAndImportDatabaseIntegrity) already operate in async contexts.Why This Works
bun:sqlite→ ERRORlocalAdapter(Node.js-compatible) → ✅localAdapterlocalAdapter(unchanged)adapterawait getAdapter()→ dynamic import → works in BunChanges
src/runtime/internal/database.server.ts— Remove static import of#content/adapter, add lazygetAdapter(), makeloadDatabaseAdapterasyncsrc/runtime/api/query.post.ts— Await the now-asyncloadDatabaseAdapterTest Plan
#content/adapterloadDatabaseAdapterreturns a workingDatabaseAdaptervia dynamic importMade with Cursor