Skip to content

sqlite: remove classes sync suffix - #65988

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

araujogui wants to merge 6 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 StatementSync to Statement.
  • Keep old names as aliases with doc-only deprecations.

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
@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

Comment thread lib/sqlite.js Outdated
@codecov

codecov Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.00000% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.35%. Comparing base (3a15c5d) to head (8a0dc69).
⚠️ Report is 108 commits behind head on main.

Files with missing lines Patch % Lines
src/node_sqlite.cc 90.64% 16 Missing ⚠️
src/node_sqlite.h 38.46% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65988      +/-   ##
==========================================
+ Coverage   90.29%   90.35%   +0.06%     
==========================================
  Files         790      790              
  Lines      272531   273835    +1304     
  Branches    52037    52358     +321     
==========================================
+ Hits       246070   247422    +1352     
+ Misses      16915    16870      -45     
+ Partials     9546     9543       -3     
Files with missing lines Coverage Δ
lib/sqlite.js 100.00% <100.00%> (ø)
src/node_sqlite.h 87.27% <38.46%> (ø)
src/node_sqlite.cc 81.98% <90.64%> (-0.08%) ⬇️

... and 65 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.

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina added the notable-change PRs with changes that should be highlighted in changelogs. label Sep 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @mcollina.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

@mcollina mcollina added the semver-minor PRs that contain new features and should be released in the next minor version. label Sep 13, 2026

@gurgunday gurgunday left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@RafaelGSS RafaelGSS added the deprecations Issues and PRs related to deprecations. label Sep 14, 2026
Comment thread doc/api/deprecations.md
@araujogui

Copy link
Copy Markdown
Member Author

@nodejs/sqlite should we rename StatementSync to Statement too?

@geeksilva97

Copy link
Copy Markdown
Contributor

@nodejs/sqlite should we rename StatementSync to Statement too?

I'd say so. It was in my plan in our first attempt.

@araujogui
araujogui force-pushed the sqlite-rename-database-class branch from 7debaa5 to 3fa9279 Compare September 14, 2026 18:17
@araujogui
araujogui force-pushed the sqlite-rename-database-class branch from 3fa9279 to 8fec2eb Compare September 22, 2026 13:06
@araujogui

Copy link
Copy Markdown
Member Author

@RafaelGSS do you still have request changes?

@trivikr
trivikr requested a review from RafaelGSS September 22, 2026 17:40

@RafaelGSS RafaelGSS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The doc-only deprecation and aliases address my concern

Existing usage continues to work without a runtime warning, so semver-minor makes sense to me. LGTM.

@araujogui araujogui added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 23, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 23, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@araujogui araujogui added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 24, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 24, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@panva panva added the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Sep 24, 2026
@github-actions github-actions Bot removed the resume-ci Add this label to resume the latest eligible Jenkins CI run on a PR with an approving review. label Sep 24, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@araujogui araujogui changed the title sqlite: rename DatabaseSync to Database sqlite: remove classes sync suffix Sep 24, 2026
@panva panva added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Sep 25, 2026
@araujogui araujogui added the commit-queue-squash PRs the Commit Queue should land as one squashed commit. label Sep 25, 2026
Rename the DatabaseSync and StatementSync classes to Database and
Statement, and the internal DatabaseSyncLimits helper to
DatabaseLimits. The old names are kept as aliases of the new classes
and are Documentation-only deprecated (DEP0210, DEP0211).

Assisted-by: Claude Code
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
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
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
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
New deprecations start out Documentation-only and only gain a runtime
warning in a later release, same as DEP0179/DEP0181 did for
crypto.Hash/Hmac. Drop the util.deprecate() wrapper for now: export
DatabaseSync as a plain alias of Database again (same function
identity, no warning), and mark DEP0210 as Documentation-only in the
docs.

Assisted-by: Claude Code
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
Signed-off-by: Guilherme Araújo <arauujogui@gmail.com>
@araujogui
araujogui force-pushed the sqlite-rename-database-class branch from 2b3ccb5 to 8a0dc69 Compare September 25, 2026 11:25
@araujogui araujogui added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 25, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 25, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

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

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash PRs the Commit Queue should land as one squashed commit. deprecations Issues and PRs related to deprecations. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.