Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat(sqlite)!: drop support for sqlite3 and default to better-sqlite3#11836

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
pkuczynski wants to merge5 commits intonext
base:next
Choose a base branch
Loading
fromsqlite

Conversation

@pkuczynski
Copy link
Collaborator

No description provided.

@qodo-free-for-open-source-projects

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 Security concerns

Sensitive information exposure:
The code contains hardcoded credentials in the template generation (lines 182-184, 192-194, 209-211, 228-230, 237-239). While these are template values for initialization and marked as "test" credentials, they could mislead developers into using weak default passwords in production environments. Consider adding comments warning users to change these credentials before deploying to production.

⚡ Recommended focus areas for review

Uninitialized Variable

The variabledbSettings is declared but not initialized before the switch statement. If the switch statement doesn't match any case (though a default is now added), TypeScript may not catch this at compile time, and it could lead to runtime errors when trying to access an undefined variable at line 263.

letdbSettings:string[]
Fall-through Behavior

The case for "sqlite" now falls through to "better-sqlite3" without an explicit return statement. While this appears intentional to map both to BetterSqlite3Driver, the lack of a break or comment indicating intentional fall-through could be confusing. Consider adding a comment to clarify this is intentional behavior.

case"sqlite":case"better-sqlite3":returnnewBetterSqlite3Driver(connection)

@qodo-free-for-open-source-projects

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: lock-file

Failed stage:Run if [ -n "$(git status --porcelain)" ]; then [❌]

Failure summary:

The action failed because thepackage-lock.json file is not up to date or was generated with a
different npm version than the one shipped with Node.js 20. The git status check detected
uncommitted changes topackage-lock.json, causing the validation script to exit with code 1.

Relevant error logs:
1:##[group]Runner Image Provisioner2:Hosted Compute Agent...164:##[endgroup]165:HEAD detached at pull/11836/merge166:Changes not staged for commit:167:(use "git add <file>..." to update what will be committed)168:(use "git restore <file>..." to discard changes in working directory)169:modified:   package-lock.json170:no changes added to commit (use "git add" and/or "git commit -a")171:##[group]Run if [ -n "$(git status --porcelain)" ]; then172:�[36;1mif [ -n "$(git status --porcelain)" ]; then�[0m173:�[36;1m  echo "package-lock.json is not up to date or was generated with a different npm version than the one shipped with Node.js $(cat .nvmrc)"�[0m174:�[36;1m  exit 1�[0m175:�[36;1mfi�[0m176:shell: /usr/bin/bash -e {0}177:##[endgroup]178:package-lock.json is not up to date or was generated with a different npm version than the one shipped with Node.js 20179:##[error]Process completed with exit code 1.180:Post job cleanup.

@qodo-free-for-open-source-projects

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                   Impact
High-level
Justify and guide this breaking change

The PR removes thesqlite3 driver, which is a significant breaking change. It
should include a justification for this removal and a migration guide for users
transitioning tobetter-sqlite3.

Examples:

src/driver/DriverFactory.ts [43]
case"sqlite":
src/driver/sqlite/SqliteDriver.ts [1-230]

Solution Walkthrough:

Before:

// PR Description is empty// src/driver/DriverFactory.tsswitch(connection.options.type){// ...case"sqlite":returnnewSqliteDriver(connection)case"better-sqlite3":returnnewBetterSqlite3Driver(connection)// ...}// src/driver/sqlite/SqliteDriver.ts// This file and its related files (options, query runner) exist.

After:

/*PR Description:# Breaking Change: Remove sqlite3 driverThis PR removes the `sqlite3` driver.**Reasoning:**The `better-sqlite3` driver offers superior performance and a simpler synchronous API, making it the recommended choice for new projects. To reduce maintenance overhead and streamline the codebase, we are deprecating and removing the older `sqlite3` driver.**Migration Guide:**1. Uninstall `sqlite3` and install `better-sqlite3`.2. In your DataSource options, change `type: "sqlite"` to `type: "better-sqlite3"`.*/// src/driver/DriverFactory.tsswitch(connection.options.type){// ...// The "sqlite" case is removed.case"better-sqlite3":returnnewBetterSqlite3Driver(connection)// ...}
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a critical process issue: the PR introduces a major breaking change by removing thesqlite3 driver without any justification or migration path for users, which severely impacts the PR's quality.

High
Possible issue
Add missing dependency for sqlite initialization

InupdatePackageJson, make thesqlite case fall through to thebetter-sqlite3
case to ensure the correct dependency is added when initializing a project with
sqlite.

src/commands/InitCommand.ts [723-726]

     ... (clipped 101 lines)                      ourPackageJson.devDependencies.pg                  break              case "sqlite":--                packageJson.dependencies["sqlite3"] =--                    ourPackageJson.devDependencies.sqlite3--                break              case "better-sqlite3":                  packageJson.dependencies["better-sqlite3"] =                      ourPackageJson.devDependencies["better-sqlite3"]  ... (clipped 39 lines)

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 9

__

Why: The PR correctly updates theinit command to generate abetter-sqlite3 configuration for thesqlite option, but it fails to add the correspondingbetter-sqlite3 dependency topackage.json, which would lead to a broken project.

High
  • More

@alumnialumni mentioned this pull requestDec 8, 2025
26 tasks
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

@pkuczynskipkuczynski

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pkuczynski

[8]ページ先頭

©2009-2025 Movatter.jp