Skip to content

sqlite: rename DatabaseSync to Database - #65988

Open
araujogui wants to merge 3 commits into
nodejs:mainfrom
araujogui:sqlite-rename-database-class
Open

sqlite: rename DatabaseSync to Database#65988
araujogui wants to merge 3 commits into
nodejs:mainfrom
araujogui:sqlite-rename-database-class

Conversation

@araujogui

@araujogui araujogui commented Sep 11, 2026

Copy link
Copy Markdown
Member

Rename the DatabaseSync class to Database, and the internal DatabaseSyncLimits helper to DatabaseLimits to match.

See #57445 (comment)

Copilot AI lite review requested due to automatic review settings September 11, 2026 17:12
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/performance
  • @nodejs/security-wg
  • @nodejs/sqlite
  • @nodejs/tsc

This comment was marked as low quality.

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Sep 11, 2026
Rename the DatabaseSync class to Database, and the internal
DatabaseSyncLimits helper to DatabaseLimits to match.

Assisted-by: claude-code:claude-sonnet-5
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
@araujogui
araujogui force-pushed the sqlite-rename-database-class branch from d90b1f9 to b52ec3e Compare September 11, 2026 17:20
@cjihrig

cjihrig commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

I didn't review the code fully, but does this leave an alias behind? If not, this will break every single current user.

@araujogui

Copy link
Copy Markdown
Member Author

I didn't review the code fully, but does this leave an alias behind? If not, this will break every single current user.

No, it doesn't. I will add it later

DatabaseSync is kept as an alias of Database, wrapped with a DEP0210
runtime deprecation warning, so existing code using the pre-rename
name keeps working.

Assisted-by: claude-code:claude-sonnet-5
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
lib/sqlite.js used object-spread (`...sqlite`) to re-export the
internal binding, which crashed tools/doc's api-links generator: it
walks module.exports's ObjectExpression properties expecting a
`value` on each one, and a SpreadElement has no such property
(TypeError: Cannot read properties of undefined (reading 'type')).
This broke the "Build from tarball" CI job.

Destructure the binding's exports into bare identifiers instead,
matching the pattern already used in lib/crypto.js for its deprecated
Hash/Hmac exports.

Also fill in the DEP0210 changes entry's pr-url, now that the pull
request exists.

Assisted-by: claude-code:claude-sonnet-5
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
Comment thread lib/sqlite.js
module.exports = {
Database,
DatabaseSync: deprecate(
Database,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I introduced a runtime deprecation of DatabaseSync but don't know if that's correct given that node:sqlite isn't stable yet.

@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.21488% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.98%. Comparing base (c143041) to head (6f49885).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/node_sqlite.h 50.00% 4 Missing ⚠️
src/node_sqlite.cc 96.73% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65988      +/-   ##
==========================================
- Coverage   90.18%   89.98%   -0.21%     
==========================================
  Files         771      784      +13     
  Lines      265487   268372    +2885     
  Branches    50461    51122     +661     
==========================================
+ Hits       239437   241496    +2059     
- Misses      17006    17426     +420     
- Partials     9044     9450     +406     
Files with missing lines Coverage Δ
lib/sqlite.js 100.00% <100.00%> (ø)
src/node_sqlite.cc 82.02% <96.73%> (-0.06%) ⬇️
src/node_sqlite.h 86.40% <50.00%> (ø)

... and 60 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants