feat: use esm only#951
Merged
Merged
Conversation
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
Review Summary by QodoModernize to ES modules with enhanced TypeScript type safety and async/await API
WalkthroughsDescription• **Modernize codebase to ES modules**: Converted from CommonJS to ESM with proper module configuration in package.json, tsconfig.json, and rolldown.config.mjs • **Remove callback-based API**: Eliminated deprecated callback support and callbackify utilities across all database methods in favor of pure async/await • **Enhance TypeScript type safety**: Added comprehensive type definitions throughout the codebase, removed @ts-nocheck directives, and enforced type-only imports using verbatimModuleSyntax • **Refactor all database drivers**: Updated 18 database driver files to use type-only imports for better ESM compatibility and cleaner output • **Improve cache layer**: Modernized CacheAndBufferLayer.ts with proper generic typing, enhanced LRU cache implementation, and improved error handling • **Define Logger interface**: Created new Logger type definition with standardized logging methods and level-checking functions • **Reorganize dependencies**: Moved database drivers to devDependencies and updated version constraints for better flexibility • **Update CI/CD**: Bumped pnpm version from 10 to 11 in GitHub Actions workflow Diagramflowchart LR
CommonJS["CommonJS<br/>Callbacks<br/>Loose Types"]
ESM["ES Modules<br/>Async/Await<br/>Strong Types"]
Config["Config Updates<br/>package.json<br/>tsconfig.json<br/>rolldown.config.mjs"]
Drivers["18 Database<br/>Drivers<br/>Type-only Imports"]
Cache["Cache Layer<br/>Generics<br/>Type Safety"]
Logger["Logger<br/>Interface<br/>Type Definitions"]
CommonJS -- "Modernize" --> ESM
ESM -- "Configure" --> Config
ESM -- "Refactor" --> Drivers
ESM -- "Enhance" --> Cache
ESM -- "Define" --> Logger
File Changes1. lib/CacheAndBufferLayer.ts
|
Code Review by Qodo
1. doBulk missing drops writes
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
539
to
551
| let success = false; | ||
| try { | ||
| await this.wrappedDB.doBulk(ops); | ||
| await this.wrappedDB!.doBulk?.(ops); | ||
| success = true; | ||
| } catch (err) { | ||
| this.logger.error( | ||
| `Bulk write of ${ops.length} ops failed, retrying individually: ${err.stack || err}`); | ||
| `Bulk write of ${ops.length} ops failed, retrying individually: ${(err as Error).stack ?? String(err)}`, | ||
| ); | ||
| this.metrics.writesToDbRetried += ops.length; | ||
| await Promise.all(ops.map(async (op, i) => await writeOneOp(op, entries[i]))); | ||
| await Promise.all(ops.map(async (op, i) => writeOneOp(op, entries[i]))); | ||
| } | ||
| if (success) entries.forEach((entry) => markDone(entry, null)); | ||
| } |
There was a problem hiding this comment.
1. Dobulk missing drops writes 🐞 Bug ≡ Correctness
CacheAndBufferLayer._write() treats a missing doBulk() as success due to optional chaining, then marks entries clean without writing anything to the underlying DB. This can silently lose buffered writes for wrapped DB implementations that do not provide doBulk().
Agent Prompt
## Issue description
`lib/CacheAndBufferLayer.ts` treats an absent `doBulk()` as a successful bulk write due to optional chaining (`doBulk?.(...)`), then marks all entries as written.
## Issue Context
`InternalDB` explicitly models `doBulk` as optional, so the cache layer must handle drivers that don't implement it by doing per-op writes.
## Fix Focus Areas
- lib/CacheAndBufferLayer.ts[40-50]
- lib/CacheAndBufferLayer.ts[536-551]
## Implementation notes
- Replace the optional-chained call with an explicit branch:
- If `this.wrappedDB!.doBulk` is a function: call it.
- Else: write individually (same logic as the retry path) and treat that as success.
- Ensure entries are only `markDone(..., null)` after at least one actual write path ran.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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.
No description provided.