Skip to content

Separate better-sqlite3 and ioredis dependencies into dedicated packages#12

Merged
badjer merged 7 commits intomainfrom
robdimarco/break-out-db-packages
Aug 21, 2025
Merged

Separate better-sqlite3 and ioredis dependencies into dedicated packages#12
badjer merged 7 commits intomainfrom
robdimarco/break-out-db-packages

Conversation

@robdimarco-atxp
Copy link
Copy Markdown
Contributor

@robdimarco-atxp robdimarco-atxp commented Aug 21, 2025

Summary

This PR separates the optional database dependencies (better-sqlite3 and ioredis) from @atxp/common into dedicated packages to reduce bundle size and dependency overhead for users who don't need these features.

New Packages Created

  • @atxp/sqlite-db: SQLite database implementation using better-sqlite3 (Node.js only)
  • @atxp/redis-db: Redis OAuth database implementation using ioredis for distributed applications

Changes Made

Dependency Separation

  • ✅ Removed better-sqlite3 and ioredis from @atxp/common dependencies
  • ✅ Removed better-sqlite3 from @atxp/client dependencies
  • ✅ Removed unused expo-sqlite peer dependency from @atxp/client
  • ✅ Created separate packages with proper TypeScript configuration

Code Migration

  • ✅ Moved SQLite platform abstractions to @atxp/sqlite-db
  • ✅ Moved Redis OAuth database implementation to @atxp/redis-db
  • ✅ Updated all tests to use MemoryOAuthDb for lightweight testing
  • ✅ Updated factory functions to guide users to appropriate packages

Release Process Updates

  • ✅ Updated GitHub Actions workflow to include new packages in version updates
  • ✅ Added new packages to NPM publishing pipeline with proper dependency ordering
  • ✅ Updated TypeScript configuration for composite builds

Documentation Updates

  • ✅ Updated main README with correct import examples and installation instructions
  • ✅ Fixed all package READMEs to accurately reflect capabilities
  • ✅ Added comprehensive READMEs for new database packages
  • ✅ Removed misleading claims about React Native support
  • ✅ Moved Redis integration test documentation to appropriate package

Migration Guide

Before (old approach)

import { SqliteOAuthDb, RedisOAuthDb } from '@atxp/common';

After (new approach)

import { MemoryOAuthDb } from '@atxp/common';  // For development
// OR install specific database packages:
import { SqliteOAuthDb } from '@atxp/sqlite-db';  // For SQLite storage
import { RedisOAuthDb } from '@atxp/redis-db';    // For Redis storage

Benefits

  • Smaller bundles: Users only install database dependencies they actually need
  • Optional dependencies: No forced installation of better-sqlite3 or ioredis
  • Better separation: Database implementations isolated from core functionality
  • Accurate documentation: READMEs now reflect actual capabilities
  • Proper release process: All packages included in automated releases

Test Results

  • ✅ All existing tests pass
  • ✅ Build process works correctly
  • ✅ TypeScript compilation successful across all packages
  • ✅ No breaking changes to public APIs (just import paths)

Package Summary

  1. @atxp/common (9 tests): Core functionality, no database dependencies
  2. @atxp/sqlite-db: SQLite implementation for Node.js applications
  3. @atxp/redis-db: Redis implementation for distributed applications
  4. @atxp/server (80 tests): Server utilities, uses MemoryOAuthDb by default
  5. @atxp/client (63 tests): Client utilities, uses MemoryOAuthDb by default

Users can now choose their database implementation based on their specific needs without carrying unnecessary dependencies.

Motivation

https://linear.app/novellum/issue/NOV-165/separate-sqlite-and-redis-dependencies-from-atxpcommon

robdimarco-atxp and others added 7 commits August 20, 2025 22:10
This change removes optional database dependencies from @atxp/common to reduce
bundle size and dependency overhead for users who don't need these features.

**New Packages:**
- @atxp/sqlite-db: SQLite database implementation with better-sqlite3
- @atxp/redis-db: Redis OAuth database implementation with ioredis

**Changes:**
- Remove better-sqlite3 and ioredis from @atxp/common dependencies
- Move SQLite platform abstractions to @atxp/sqlite-db
- Move Redis OAuth database code to @atxp/redis-db
- Update tests to use MemoryOAuthDb for in-memory testing
- Update factory functions to guide users to appropriate packages
- Remove SQLite-specific platform tests from common package

**Migration:**
- For SQLite: import from '@atxp/sqlite-db'
- For Redis: import from '@atxp/redis-db'
- Core functionality remains in '@atxp/common'

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add @atxp/sqlite-db and @atxp/redis-db to version update process
- Add new packages to NPM publishing pipeline
- Update TypeScript configuration for proper composite builds
- Ensure dependency version synchronization across all packages

The release process now handles all 5 packages:
1. @atxp/common (core functionality)
2. @atxp/sqlite-db (SQLite database implementation)
3. @atxp/redis-db (Redis database implementation)
4. @atxp/server (server utilities)
5. @atxp/client (client utilities)

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
**README Updates:**
- Fix main README to show correct import paths for new database packages
- Update @atxp/common README to reference separate database packages
- Update @atxp/client README to clarify platform support
- Add comprehensive READMEs for @atxp/sqlite-db and @atxp/redis-db

**Dependency Cleanup:**
- Remove better-sqlite3 dependency from @atxp/client package
- Update package-lock.json to reflect dependency changes

**Migration Guidance:**
- Clear instructions for upgrading from old SQLite/Redis usage
- Examples showing how to use new separate packages
- Platform-specific installation instructions

Users now have clear guidance on:
- Using MemoryOAuthDb for development
- Installing @atxp/sqlite-db for persistent storage
- Installing @atxp/redis-db for distributed applications

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
**Dependency Cleanup:**
- Remove `expo-sqlite` peer dependency from @atxp/client (not actually used)
- Remove expo-sqlite mock from test setup (no longer needed)
- Update package-lock.json to remove unused dependencies

**Documentation Fixes:**
- Fix @atxp/sqlite-db README to clarify it's Node.js only (no React Native support)
- Update @atxp/client README to remove references to expo-sqlite
- Update @atxp/common README to accurately describe SQLite package scope
- Move REDIS_INTEGRATION_TESTS.md to @atxp/redis-db package

**Clean Up Build Artifacts:**
- Remove Redis build artifacts from @atxp/common dist/

**Corrected Claims:**
- @atxp/sqlite-db: Node.js only (better-sqlite3)
- @atxp/redis-db: Distributed storage with ioredis
- React Native: Use MemoryOAuthDb for now (persistent storage not implemented)

The packages now accurately represent their capabilities and dependencies.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
**SQLite Package (@atxp/sqlite-db):**
- ✅ 26 tests covering full CRUD operations
- ✅ Platform compatibility tests (bundler safety)
- ✅ Integration tests with real SQLite database files
- ✅ Encryption support tests
- ✅ Database schema validation
- ✅ Error handling and edge cases
- ✅ Concurrent access testing

**Redis Package (@atxp/redis-db):**
- ✅ 16 unit tests with mocked Redis client
- ✅ 10 integration tests (run with REDIS_URL env var)
- ✅ TTL behavior testing
- ✅ Key management and prefixing
- ✅ Encryption support tests
- ✅ Error handling and connection failures

**Test Coverage:**
- All database operations (client credentials, PKCE values, access tokens)
- Custom encryption/decryption functions
- TTL and expiration behavior
- Platform-specific functionality
- Error conditions and edge cases
- Real database integration scenarios

**Moved Tests:**
- Moved bundler compatibility tests from @atxp/common to @atxp/sqlite-db
- Removed duplicate test files from common package

All packages now have proper test coverage and CI will pass.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Removed unused variables:
- Unused error parameter in catch block
- Unused invalidPath variable in error handling test

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@robdimarco-atxp robdimarco-atxp requested a review from badjer August 21, 2025 03:09
Copy link
Copy Markdown
Contributor

@badjer badjer left a comment

Choose a reason for hiding this comment

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

This looks really solid @robdimarco-novellumai - thanks for taking this on!

My big request is that we rename these new packages before we publish the packages - all my other comments are non-blocking. That said, I'm going to merge this straight in because having it in main unblocks me :)

# Update all workspace package versions
npm version $VERSION --no-git-tag-version -w packages/atxp-common
npm version $VERSION --no-git-tag-version -w packages/atxp-sqlite-db
npm version $VERSION --no-git-tag-version -w packages/atxp-redis-db
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we call these atxp-sqlite and atxp-redis instead? Apologies for the pedantry, but those are slightly nicer names, and it leaves the door open to other sqlite / redis dependencies fitting into these packages in the future, which is I think the cut-point we want here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll have a fast follow with the rename.

// - @atxp/redis-db for Redis support
//
// This file remains for backwards compatibility and provides factory functions
// that guide users to the appropriate packages. No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not tracking with this - I don't think keeping this file really provides any value?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree, bad bot.

* Note: SQLite and Redis implementations have been moved to separate packages.
* This factory now only supports the MemoryOAuthDb implementation.
* For SQLite support, import from '@atxp/sqlite-db'
* For Redis support, import from '@atxp/redis-db'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should probably just scrap this file at this point? I don't think there's a lot of value in a factor that can only build one thing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed.

import { describe, it, expect } from 'vitest';
import { SqliteOAuthDb } from './oAuthDb.js';

describe('SqliteOAuthDb (Expo)', () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this test probably moves to the new sqlite package - that way we're still testing that we can load SQLite on multiple platforms.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense.

},
"dependencies": {
"@atxp/common": "^0.2.10",
"ioredis": "^5.7.0"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FYI in longrun we're using iovalkey instead. It looks like ioredis is much more popular and more actively developed, and I don't have any FUD about using redis. But flagging because we should probably have a discussion about standardizing on one or the other.

Non-blocking and beyond the scope of this PR, obviously.

@badjer badjer merged commit d184b78 into main Aug 21, 2025
1 check passed
@badjer badjer deleted the robdimarco/break-out-db-packages branch August 21, 2025 15:43
robdimarco-atxp added a commit that referenced this pull request Aug 21, 2025
- Rename packages from atxp-sqlite-db to atxp-sqlite and atxp-redis-db to atxp-redis
- Remove oAuthDb.ts and oAuthDbFactory.ts files as they are no longer needed
- Update all documentation and references to use new package names
- Replace factory pattern with direct database instance creation

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
robdimarco-atxp added a commit that referenced this pull request Aug 21, 2025
* Implement Brian's feedback from PR #12

- Rename packages from atxp-sqlite-db to atxp-sqlite and atxp-redis-db to atxp-redis
- Remove oAuthDb.ts and oAuthDbFactory.ts files as they are no longer needed
- Update all documentation and references to use new package names
- Replace factory pattern with direct database instance creation

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Fix package-lock.json and main README references

- Update main README.md to reference new package names (@atxp/sqlite instead of @atxp/sqlite-db)
- Regenerate package-lock.json to resolve npm ci failures

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Fix build script references to renamed packages

- Update build script to use new package names (atxp-sqlite, atxp-redis)
- Update test:integration script to reference correct package

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

2 participants