Skip to content

Conversation

@geoffreyfarrel
Copy link
Contributor

@geoffreyfarrel geoffreyfarrel commented Oct 21, 2025

Feature: Standardized Storage Module (Adapter Pattern)

This PR introduces a storage integration module using the Adapter Pattern, ensuring all storage backends (GCS, S3, R2, Azure Blob, Local) present a unified, type-safe API to the rest of the application, drastically improving testability, dependency management, and type safety.

Key Features and Improvements

StorageBaseModule Enhancements

The module is now a fully featured NestJS Dynamic Module, supporting dependency injection for configuration:

  • forRoot<A>(options) (Synchronous): Standard setup for local or static configuration with full type inference
  • forRootAsync<A>(options) (Asynchronous): Enables configuration that depends on other modules (e.g., ConfigService, database access) via useFactory, useClass, or useExisting. This allows us to fetch credentials and bucket names at runtime
  • Generic Type Preservation: The module preserves adapter type information through the DI chain using InstanceType<A>, enabling type-safe access to adapter-specific APIs

Standardized Contract (IStorageAdapter)

A new TypeScript interface (IStorageAdapter) defines the strict contract for all storage providers:

  • Type Safety: Ensures all public methods (write, remove, isExists, batchWrite) have consistent signatures
  • Flexible URL Generation: Optional url() method with flexible signature (url?(key: string, ...args: any[]): Promise<string>) to support varying adapter requirements (GCS: url(key, expires?), S3: url(key), R2: url(key, options?))
  • Encapsulation: Adapter-specific logic remains in adapter implementations, not in the base module

StorageService (The Public API)

The service is now a generic, type-safe business logic layer:

  • Generic Type Support: StorageService<A extends IStorageAdapter> preserves adapter type information for full type inference
  • Type-Safe URL Method: Uses TypeScript conditional types to extract parameter and return types from the adapter's url() signature:
    type ParametersOfUrl<A extends IStorageAdapter> =
      NonNullable<A['url']> extends (...params: infer P) => unknown ? P : never;
    
    type ReturnTypeOfUrl<A extends IStorageAdapter> =
      NonNullable<A['url']> extends (...params: unknown[]) => infer R ? R : never;
  • Unified API: Provides clean method calls (e.g., url(key: string, expires: number)) with correct type inference based on the adapter
  • Common Options Handling: Correctly injects MaxFileSizeInBytes, defaultPublic, etc., and applies these checks (e.g., file size validation) before calling the adapter
  • Dependency Injection: The constructor now only relies on the abstract contract (@Inject(STORAGE_ADAPTER) private readonly _adapter: A), making it completely independent of any concrete storage implementation

Optional Dependencies Architecture

  • Lean Dependencies: All adapter packages (@rytass/storages-adapter-*) are declared as optionalDependencies
  • Flexible Installation: Users only install the adapters they need, reducing bundle size and dependency overhead
  • No Forced Dependencies: The base module only requires @rytass/storages core package

Type-Safe URL Generation

The url() method implementation eliminates runtime type checks:

  • No constructor.name checks: Removed unsafe runtime type checking
  • No if/switch conditionals: Uses TypeScript generics and conditional types instead
  • Compile-time type safety: TypeScript enforces correct parameter usage for each adapter type
  • Autocomplete support: Full IDE support for adapter-specific method signatures

Testing & Quality Assurance

  • Complete Test Suite: Includes a robust set of tests covering both module setup and service functionality
  • Unit Tests: Tests module initialization, configuration merging/defaulting logic, and all service method logic
  • Integration Tests: Verifies that the module successfully compiles and interacts with real adapter classes (e.g., StorageGCSService), ensuring real-world compatibility
  • Async Coverage: All forRootAsync patterns (useFactory, useClass, useExisting) are covered
  • Type Safety Tests: Verifies correct type inference for different adapter url() signatures (GCS, S3, R2, Local)

Usage Example

import { StorageBaseModule } from '@rytass/storages-base-nestjs-module';
import { StorageGCSService } from '@rytass/storages-adapter-gcs';

@Module({
  imports: [
    StorageBaseModule.forRoot({
      adapter: StorageGCSService,
      config: {
        bucket: 'my-bucket',
        projectId: 'my-project',
        credentials: {
          client_email: 'test@example.com',
          private_key: '-----BEGIN PRIVATE KEY-----...'
        }
      },
      commonOptions: {
        MaxFileSizeInBytes: 10 * 1024 * 1024,
        defaultPublic: false
      }
    })
  ]
})
export class AppModule {}

// Type-safe usage in services
@Injectable()
export class MyService {
  constructor(
    private readonly storage: StorageService<StorageGCSService>
  ) {}
  
  async getUrl(key: string, expires: number) {
    // TypeScript knows this adapter accepts (key: string, expires?: number)
    return await this.storage.url(key, expires);
  }
}

Benefits

  • Type Safety: Full TypeScript inference for adapter-specific methods
  • Reduced Bundle Size: Users only install adapters they need
  • Better Developer Experience: Autocomplete and type checking for adapter-specific APIs
  • Maintainability: No wrapper classes or runtime type checks
  • Flexibility: Supports any adapter implementing IStorageAdapter with varying method signatures
  • Testability: Easy to mock and test with generic type support

Breaking Changes

  • Wrapper classes removed: Users must use adapter classes directly (e.g., StorageGCSService instead of GCSAdapter)
  • Optional dependencies: Users must explicitly install adapter packages they need
  • Generic types required: Must specify adapter type when using StorageService for full type safety

Migration Guide

Before:

import { StorageBaseModule, S3Adapter } from '@rytass/storages-base-nestjs-module';

@Module({
  imports: [
    StorageBaseModule.forRoot({
      adapter: S3Adapter, // wrapper class
      config: { ... }
    })
  ]
})
export class AppModule {}

After:

import { StorageBaseModule } from '@rytass/storages-base-nestjs-module';
import { StorageS3Service } from '@rytass/storages-adapter-s3'; // install separately

@Module({
  imports: [
    StorageBaseModule.forRoot({
      adapter: StorageS3Service, // direct adapter class
      config: { ... }
    })
  ]
})
export class AppModule {}

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Oct 21, 2025

Deploying utils-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4fe5a88
Status: ✅  Deploy successful!
Preview URL: https://7673b047.utils-docs.pages.dev
Branch Preview URL: https://feat-storage-nestjs-module.utils-docs.pages.dev

View logs

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Oct 21, 2025

Deploying rytass-utils-storybook with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4fe5a88
Status: ✅  Deploy successful!
Preview URL: https://c1a9d577.rytass-utils-storybook.pages.dev
Branch Preview URL: https://feat-storage-nestjs-module.rytass-utils-storybook.pages.dev

View logs

@fantasywind
Copy link
Member

Overall, the current implementation of this module causes users to install packages they don’t need. This goes against the original intent of the module. Please consider an approach where all modules are declared as optionalDependencies.

Since every module follows the storages root module’s contract, you should treat them all as root modules and expose optional configuration via generic types so users can inject their own types to change/infer the TypeScript tokens. In other words, the implementations inside those wrappers shouldn’t be there at all.

Additionally, using constructor.name for type checks is very unsafe and should not be used that way.

Please think more deeply about the role generics play in TypeScript and in this module’s design, and avoid relying on if/switch-style conditionals to implement it.

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