feat: implement CDN provider strategy pattern

[doong-jo]

Summary

Major updates across the entire codebase:

Core Package Enhancements:

• Add CDN provider strategy pattern with pluggable architecture
• Implement url-builder-factory.ts with strategy-based URL building
• Add constants.ts for CDN provider configurations
• Add comprehensive CDN provider tests (cdn-provider.test.ts)
• Add basic-usage.ts and environment-setup.md examples

React Package Test Fixes:

• Fix failing hydration-safe-dpr.test.ts by standardizing mock structure
• Resolve "imageEngine.generateImageData is not a function" errors
• All React package tests now pass: 46/46 ✅
• Improve test reliability and consistency

Next.js and React Demo Updates:

• Add CdnConfigurationExample.tsx to both demo apps
• Update environment configurations (.env.local, .env)
• Enhance demo app package.json dependencies

Documentation Improvements:

• Comprehensive README updates for core, react, and nextjs packages
• Enhanced API documentation and usage examples
• Better developer onboarding guides

Infrastructure:

• Update CI workflow (.github/workflows/ci.yml)
• Optimize turbo.json configuration
• Update pnpm-lock.yaml with new dependencies

Type Safety & Architecture:

• Improved TypeScript types and interfaces
• Better environment configuration handling
• Simplified image engine architecture
• Enhanced error handling and validation

Breaking Changes:

• CDN provider configuration API updates
• Modified URL builder interface for strategy pattern

Test Coverage:
✅ React Tests: 46/46 passing
✅ Core Tests: Comprehensive CDN provider coverage
✅ E2E Tests: Enhanced nextjs-demo test suite

Checklist

• PR title follows conventional commits format (see format guide)
• Documentation updated (if applicable)
• No breaking changes (unless intentional and documented)

Screenshots (if applicable)

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
snapkit-js-nextjs-demo	Ready	Preview	Comment	Sep 29, 2025 8:29am
snapkit-js-react-demo	Ready	Preview	Comment	Sep 29, 2025 8:29am

[doong-jo]

@claude

[claude]

Claude finished @doong-jo's task —— View job

---

🔍 PR Review: CDN Provider Strategy Pattern

✅ Strengths

Architecture & Design

• Excellent strategy pattern implementation for CDN providers in /packages/core/src/env-config.ts:20-106 - clean separation of concerns between Snapkit and custom CDN providers
• Smart factory pattern in UrlBuilderFactory (/packages/core/src/url-builder-factory.ts:8-54) prevents unnecessary instance creation with proper caching
• Well-organized constants in /packages/core/src/constants.ts:1-56 centralizes configuration values effectively

Code Quality

• Comprehensive test coverage - 204 lines of CDN provider tests, extensive factory tests with edge cases
• Type safety throughout with proper TypeScript interfaces (CdnConfig, CdnProvider, EnvironmentStrategy)
• Good error handling with descriptive messages (e.g., lines 84-91 in cdn-provider.test.ts)

Environment Flexibility

• Multi-framework support - automatic detection for Vite, CRA, Next.js, and Node.js environments
• Universal strategy fallback ensures the library works in any environment

⚠️ Areas for Improvement

1. Breaking Changes Impact 🔴

• The CDN provider configuration API changes could break existing implementations
• Consider adding a migration guide or compatibility layer for smoother transition
• The PR doesn't clearly document migration paths for existing users

2. Error Handling Enhancement

// In /packages/core/src/url-builder.ts:9-24
constructor(config: CdnConfig) {
  if (config.provider === 'snapkit') {
    if (!config.organizationName) {
      throw new Error(
        'organizationName is required when using snapkit provider',
      );
    }
    // Consider validating organizationName format (e.g., no spaces, special chars)
    this.baseUrl = `https://${config.organizationName}-cdn.snapkit.studio`;
  }
}

3. Factory Pattern Memory Concerns

// In url-builder-factory.ts
// The cache never expires - could lead to memory leaks in long-running applications
private static instances = new Map<string, SnapkitUrlBuilder>();

// Consider adding TTL or max cache size:
// - Implement LRU cache with size limit
// - Add timestamp-based expiration

4. Environment Detection Edge Cases

// In env-config.ts:44-50
detect: () => {
  return (
    typeof import.meta !== 'undefined' &&
    // @ts-expect-error
    import.meta.env?.MODE !== undefined
  );
},
// This could fail in some edge cases - consider more robust detection

📝 Suggestions

1. Add Validation for Custom URLs

// In url-builder.ts, validate custom baseUrl:
if (config.provider === 'custom') {
  if (!config.baseUrl) {
    throw new Error('baseUrl is required when using custom provider');
  }
  // Add URL validation
  try {
    new URL(config.baseUrl);
  } catch {
    throw new Error('Invalid baseUrl: must be a valid URL');
  }
  this.baseUrl = config.baseUrl;
}

2. Improve Test Documentation

• Add comments explaining the test scenarios in /packages/core/src/__tests__/cdn-provider.test.ts
• Include examples of expected vs actual URLs for clarity

3. Consider Adding Telemetry

// Track which CDN providers are being used (opt-in)
interface CdnConfig {
  provider: CdnProvider;
  organizationName?: string;
  baseUrl?: string;
  telemetry?: boolean; // Optional telemetry for usage analytics
}

4. Documentation Improvements

• Add a migration guide for the breaking changes
• Include performance benchmarks comparing different CDN providers
• Add troubleshooting section for common CDN configuration issues

🎯 Minor Issues

1. Korean comments in test file (line 88-89, 98-99 in cdn-provider.test.ts) - consider using English for consistency
2. Missing JSDoc for some public methods in UrlBuilderFactory
3. Inconsistent error messages - some include parameter names, others don't

✨ Overall Assessment

This is a well-architected implementation of the CDN provider strategy pattern that significantly improves the flexibility of the library. The code quality is high, tests are comprehensive, and the pattern implementations are clean.

Recommended Actions:

1. ✅ Add migration guide for breaking changes
2. ✅ Consider implementing cache management for UrlBuilderFactory
3. ✅ Validate custom CDN URLs
4. ⚠️ Review environment detection robustness
5. 📝 Enhance documentation with troubleshooting guides

The PR is technically sound and ready for merge after addressing the migration guide for breaking changes. Great work on the strategy pattern implementation! 🚀

---

Review performed by Claude on snapkit-studio/snapkit-js PR #3

[claude]

🔍 Code Review for PR #3: CDN Provider Strategy Pattern

Great work on implementing the CDN provider strategy pattern! This is a well-architected solution that provides excellent flexibility for both Snapkit CDN and custom CDN providers. The implementation shows strong TypeScript practices and comprehensive test coverage.

✅ Strengths

• Clean Architecture: Excellent use of Factory and Strategy patterns with proper separation of concerns
• TypeScript Excellence: Strong type safety throughout with well-defined interfaces
• Framework Agnostic: Works seamlessly across Next.js, React, and Vite environments
• Test Coverage: Comprehensive test suite including the React hydration fixes (46/46 passing) ✨
• Developer Experience: Clear examples and documentation updates make adoption straightforward

🚨 High Priority Issues

1. Security: URL Validation Missing

// packages/core/src/url-builder.ts line 32-34
if (src.startsWith('http://') || src.startsWith('https://')) {
  return src; // No validation - potential security risk
}

Recommendation: Add URL validation to prevent malicious URLs from being passed through.

2. Security: Path Traversal Risk

// packages/core/src/url-builder.ts line 37
const path = src.startsWith('/') ? src : `/${src}`;

Recommendation: Sanitize paths to prevent ../ attacks:

const sanitizePath = (path: string) => path.replace(/\.\./g, '').replace(/\/+/g, '/');

⚠️ Performance Considerations

1. Cache Management

The UrlBuilderFactory cache has no size limits, which could lead to memory issues in long-running applications.

Recommendation: Implement LRU cache with configurable limits:

private static readonly MAX_CACHE_SIZE = 50;

2. Query Parameter Optimization

Consider caching frequently used transform combinations to avoid rebuilding query strings.

🔧 Code Quality Improvements

1. Error Handling Enhancement

// Add more context to error messages
throw new Error(`CDN configuration error: ${context} - ${details}`);

2. Environment Detection Robustness

The Vite environment detection could be more reliable:

detect: () => {
  return typeof import.meta !== 'undefined' && 
         import.meta.env?.MODE !== undefined &&
         typeof import.meta.env.VITE_IMAGE_CDN_PROVIDER !== 'undefined';
}

📋 Test Coverage Gaps

While the test coverage is good, consider adding:

• Security test cases (path traversal, XSS prevention)
• Performance tests (cache memory usage, concurrent access)
• Edge cases (special characters, very long URLs, malformed env vars)

💡 Suggestions for Enhancement

1. Configuration Validation: Validate CDN URLs are reachable during initialization (optional)
2. Monitoring Hooks: Add optional performance metrics collection
3. Graceful Degradation: Fallback behavior when CDN is unreachable

📝 Documentation Notes

The README updates are excellent! Consider adding:

• Migration guide from previous versions
• Troubleshooting section for common CDN configuration issues
• Performance tuning recommendations

✨ Overall Assessment

Score: 8.5/10

This is a solid implementation that successfully achieves the goal of supporting flexible CDN configuration. The architecture is clean, maintainable, and well-tested. The main improvements needed are around security hardening and performance optimization for production environments.

The breaking changes are well-documented, and the migration path seems reasonable. Great job on maintaining backward compatibility while adding this powerful new feature!

---

🤖 Generated with Claude Code

Co-Authored-By: Claude [email protected]