Skip to content

Update better-sqlite3 to v12.4.4 for Node.js 24 support#230

Open
IanMayo wants to merge 8 commits intothevahidal:mainfrom
DeepBlueCLtd:claude/update-better-sqlite3-012SY7Fi8SFrJuYqn6a2RKWV
Open

Update better-sqlite3 to v12.4.4 for Node.js 24 support#230
IanMayo wants to merge 8 commits intothevahidal:mainfrom
DeepBlueCLtd:claude/update-better-sqlite3-012SY7Fi8SFrJuYqn6a2RKWV

Conversation

@IanMayo
Copy link
Collaborator

@IanMayo IanMayo commented Nov 16, 2025

  • Upgrade better-sqlite3 from ^8.1.0 to ^12.4.4
  • Enables Node.js 24 compatibility
  • better-sqlite3 v12.x supports Node.js 20, 22, 23, and 24
  • No breaking API changes affecting soul-cli functionality

Fixes compatibility issues when running on Node.js 24.

Test verification:

 npx cross-env \
    CI=true \
    NODE_ENV=test \
    NO_CLI=true \
    DB=test.db \
    CORE_PORT=8001 \
    AUTH=true \
    INITIAL_USER_USERNAME=admin \
    INITIAL_USER_PASSWORD=Admin123! \
    TOKEN_SECRET=test-secret-key-min-10-chars \
    jest --testTimeout=10000 --runInBand
(node:99545) ExperimentalWarning: CommonJS module /Users/ian/.nvm/versions/node/v23.2.0/lib/node_modules/npm/node_modules/debug/src/node.js is loading ES Module /Users/ian/.nvm/versions/node/v23.2.0/lib/node_modules/npm/node_modules/supports-color/index.js using require().
Support for loading ES Module in require() is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
Test suite started
Creating test table...
Inserting a row into test table...
PASS src/controllers/auth.test.js
  ● Console

    console.log
      No extensions directory provided

      at log (src/extensions.js:41:13)

    console.log
      Initial user created successfully

      at log (src/controllers/auth/user.js:381:15)

PASS src/controllers/tables.test.js
  ● Console

    console.log
      Initial user is already created

      at log (src/controllers/auth/user.js:383:15)

    console.log
      No extensions directory provided

      at log (src/extensions.js:41:13)

PASS src/controllers/rows.test.js
  ● Console

    console.log
      Initial user is already created

      at log (src/controllers/auth/user.js:383:15)

    console.log
      No extensions directory provided

      at log (src/extensions.js:41:13)

PASS src/controllers/index.test.js
  ● Console

    console.log
      Initial user is already created

      at log (src/controllers/auth/user.js:383:15)

    console.log
      No extensions directory provided

      at log (src/extensions.js:41:13)


Test Suites: 4 passed, 4 total
Tests:       35 passed, 35 total
Snapshots:   0 total
Time:        2.095 s, estimated 3 s
Ran all test suites.
Test suite finished
Dropping test database...
successfully deleted test.db
Jest did not exit one second after the test run has completed.

This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.

claude and others added 2 commits November 16, 2025 10:14
- Upgrade better-sqlite3 from ^8.1.0 to ^12.4.1
- Enables Node.js 24 compatibility
- better-sqlite3 v12.x supports Node.js 20, 22, 23, and 24
- No breaking API changes affecting soul-cli functionality

Fixes compatibility issues when running on Node.js 24.
@IanMayo IanMayo marked this pull request as draft November 16, 2025 10:31
@IanMayo IanMayo changed the title Update better-sqlite3 to v12.4.1 for Node.js 24 support Update better-sqlite3 to v12.4.4 for Node.js 24 support Nov 16, 2025
@IanMayo IanMayo marked this pull request as ready for review November 16, 2025 12:11
Add AUTH, TOKEN_SECRET, and initial user credentials to test script
so auth-related tests can run properly. Add --forceExit for clean exit.
Prevents the setInterval from blocking process exit during tests.
Handle SQLITE_CONSTRAINT_UNIQUE error when default role already exists,
which can occur when Jest workers share the test database but have
separate module caches.
Copy link
Owner

@thevahidal thevahidal left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @IanMayo, I left some comments for you.

package.json Outdated
Copy link
Owner

Choose a reason for hiding this comment

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

Please bump the Soul version as well

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

BTW - I haven't forgotten this PR @thevahidal , but I started a new job in December, and it has stolen my Open Source contribution time. Hopefully things will settle down soon enough for me to fix these bits :-D

if (e.code !== 'SQLITE_CONSTRAINT_UNIQUE') {
throw e;
}
// role already exists, ignore
Copy link
Owner

Choose a reason for hiding this comment

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

It the role already exists, shouldn't we get it from db?

// role already exists, ignore
roleId = rowService...

"cli": "nodemon src/server.js --database foobar.db",
"swagger-autogen": "cross-env NO_CLI=true node src/swagger/index.js",
"test": "cross-env CI=true NODE_ENV=test NO_CLI=true DB=test.db CORE_PORT=8001 jest --testTimeout=10000",
"test": "cross-env CI=true NODE_ENV=test NO_CLI=true DB=test.db CORE_PORT=8001 AUTH=true TOKEN_SECRET=test-secret INITIAL_USER_USERNAME=admin INITIAL_USER_PASSWORD=Admin123#Test jest --testTimeout=10000 --forceExit",
Copy link
Owner

Choose a reason for hiding this comment

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

For now it's ok but in the future we need to check why we need --forceExit and fix any related issues.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants