Skip to content

v3 last review#31

Merged
tinsever merged 40 commits intov3-pre-pushfrom
v3-dev
Jan 1, 2026
Merged

v3 last review#31
tinsever merged 40 commits intov3-pre-pushfrom
v3-dev

Conversation

@tinsever
Copy link
Owner

@tinsever tinsever commented Jan 1, 2026

Greptile Summary

This PR represents a major v3.0.0 release that modernizes the google-font-cli codebase with significant improvements to reliability, maintainability, and user experience.

Key Changes:

  • Modernized codebase from callbacks to async/await patterns throughout all core modules
  • Added comprehensive TypeScript type definitions and JSDoc annotations for better type safety
  • Implemented 24-hour caching system for Google Font list to reduce API calls and improve performance
  • Added batch operation support allowing multiple fonts to be downloaded/installed in a single command
  • Improved error handling with AggregateError for partial failures, allowing operations to continue even when some variants fail
  • Enhanced variant normalization supporting 'normal' as an alias for '400' and 'normalitalic' for '400italic'
  • Added comprehensive Jest test suite with 100+ unit tests covering all core modules
  • Updated dependencies to latest versions (commander ^12, file-type ^19, react ^19, etc.)
  • Improved request timeout handling (increased to 10s) and added MIME type validation for downloaded fonts
  • Added --refresh-cache flag and format options (--ttf, --woff2) for better CLI control

Quality Improvements:

  • Extracted magic strings into named constants (DEFAULT_FORMAT)
  • Better error messages and debugging with error.cause properties
  • Promise deduplication in font list loading to prevent race conditions
  • Best-effort error handling in non-critical paths (cache operations)

Confidence Score: 4/5

  • This PR is safe to merge with minor observations
  • The PR demonstrates solid engineering practices with comprehensive test coverage, proper error handling, and backward compatibility. The code changes are well-structured and modernize the codebase significantly. Score reduced from 5 to 4 due to one notable observation: the variant normalization logic adds aliases but the implementation could be clearer about the complete mapping strategy.
  • Pay attention to lib/google-font.js for the error handling flow with AggregateError and variant normalization logic

Important Files Changed

Filename Overview
lib/google-font.js Modernized with async/await, added TypeScript annotations, improved error handling with AggregateError, and enhanced variant normalization
lib/system-font.js Converted to async/await pattern, improved error handling with cause property, added comprehensive JSDoc documentation
lib/cache.js New cache module implementing 24-hour TTL for Google Font list with best-effort error handling
lib/request.js Refactored with modern URL API, improved timeout handling (10s), added MIME type detection for downloaded fonts
cli.js Added batch operation support, improved error handling for partial failures, implemented cache refresh functionality
lib/google-font-list.js Added cache integration, async/await refactor, improved concurrent load handling with promise deduplication
lib/types.d.ts New comprehensive TypeScript definitions for all modules, interfaces, and callbacks
package.json Updated dependencies to latest versions, added Jest test scripts and TypeScript type checking

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI
    participant FontList
    participant Cache
    participant GoogleFont
    participant SystemFont
    participant Request
    participant GWFH API

    User->>CLI: gfcli download "Roboto, Open Sans"
    CLI->>FontList: ensureFontsLoaded(refresh)
    
    alt Cache enabled and valid
        FontList->>Cache: readCache()
        Cache-->>FontList: cached fonts
        FontList->>FontList: populate(cached)
    else Cache miss or expired
        FontList->>Request: new Request(gwfhFontApiUrl)
        Request->>GWFH API: GET /api/fonts
        GWFH API-->>Request: font list JSON
        Request-->>FontList: parseRawData(data)
        FontList->>Cache: writeCache(fonts)
    end
    
    FontList-->>CLI: fonts loaded
    
    loop For each family
        CLI->>FontList: getFontByName(term)
        FontList-->>CLI: filteredList
        CLI->>GoogleFont: getFirst()
        GoogleFont-->>CLI: font instance
        
        CLI->>GoogleFont: saveAtAsync(variants, dest, format)
        GoogleFont->>GoogleFont: _getFileMapAsync(format)
        GoogleFont->>GWFH API: GET /api/fonts/{family}
        GWFH API-->>GoogleFont: variant URLs
        
        loop For each variant
            GoogleFont->>SystemFont: saveAt(url, dest, fileName)
            SystemFont->>SystemFont: _saveTmp(url, fileName)
            SystemFont->>Request: new Request(url)
            Request->>GWFH API: GET font file
            GWFH API-->>Request: font file data
            Request->>Request: validate MIME type
            Request-->>SystemFont: tmpPath
            SystemFont->>SystemFont: _move(tmpPath, destFolder)
            SystemFont-->>GoogleFont: final path
        end
        
        alt All variants succeed
            GoogleFont-->>CLI: FontResult[]
        else Some variants fail
            GoogleFont-->>CLI: throw AggregateError with partial results
        end
    end
    
    CLI->>CLI: printResult(allResults)
    CLI-->>User: Success message with paths
Loading

Tin Sever and others added 30 commits December 31, 2025 17:51
- Remove async library dependency, replace with native async/await
- Remove mv package, use native fs.promises.rename()
- Update commander v2 -> v12 (latest)
- Update file-type v3 -> v19 (latest)
- Convert all callback-based code to async/await in:
  - lib/system-font.js
  - lib/google-font.js
  - lib/google-font-list.js
  - lib/request.js
- Maintain backward compatibility with callback APIs
- Use native Array methods instead of async library
- Use fs.promises instead of callback-based fs
- Update CLI to use new Commander API

BREAKING CHANGES:
- Requires Node.js 14+ for fs.promises and recursive mkdir
- Commander v12 requires new Command() syntax
feat: modernize codebase with async/await and updated dependencies
- add filesystem cache (24h TTL) for Google font list
- support --refresh-cache to force reload
- allow batch install/download for multiple families
- better error handling during lookup and install/download flows
- add helper functions for parsing family lists
- drop async package usage in font list and font handling (native loops)
- add explicit refresh message when using --refresh-cache
- update README for caching/batch usage
- misc gitignore tweak
- Add Jest test framework with 127 passing tests
- Add test coverage for all library modules:
  - cache.js (100% coverage)
  - noop.js (100% coverage)
  - google-font-list.js (93% coverage)
  - request.js (93% coverage)
  - google-font.js
  - system-font.js
- Add jest.config.js with coverage configuration
- Add test scripts: test, test:watch, test:coverage
feat: add comprehensive test suite with Jest
- Add tsconfig.json with allowJs/checkJs for type checking without compilation
- Add lib/types.d.ts with shared type definitions (FontData, FontResult, etc.)
- Add lib/vendor.d.ts for untyped packages (copy-paste-win32fix, node-powershell)
- Annotate all lib/*.js files with JSDoc type comments
- Annotate cli.js with type annotations
- Add typescript and @types/node devDependencies
- Add 'typecheck' npm script
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
[WIP] Address feedback on JSDoc type annotations
feat: add JSDoc type annotations and TypeScript type checking
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: tinsever <176929823+tinsever@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: tinsever <176929823+tinsever@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…es only after operation completes

Co-authored-by: tinsever <176929823+tinsever@users.noreply.github.com>
@greptile-apps
Copy link

greptile-apps bot commented Jan 1, 2026

Greptile's behavior is changing!

From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section.

This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR".

@tinsever tinsever merged commit 09e0142 into v3-pre-push Jan 1, 2026
1 check passed
@tinsever tinsever deleted the v3-dev branch January 1, 2026 17:47
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