Skip to content

Fix/google font cli cleanup#30

Merged
tinsever merged 6 commits intov3-devfrom
fix/google-font-cleanup
Jan 1, 2026
Merged

Fix/google font cli cleanup#30
tinsever merged 6 commits intov3-devfrom
fix/google-font-cleanup

Conversation

@tinsever
Copy link
Owner

@tinsever tinsever commented Jan 1, 2026

Greptile Summary

Cleaned up lib/google-font.js with several refactoring improvements and bug fixes:

  • Removed unused util import
  • Converted var to const for better scoping
  • Extracted DEFAULT_FORMAT constant to eliminate magic string 'ttf' repetition
  • Enhanced _normalizeVariant() to support 'normal' and 'normalitalic' aliases in addition to '400' and '400italic'
  • Changed error handling in saveAtAsync() to collect errors in an array attached to the result rather than silently discarding them

The changes improve code maintainability and make error handling more transparent, though the .errors property added to the array return type isn't reflected in the TypeScript definitions.

Confidence Score: 4/5

  • This PR is safe to merge with low risk - primarily refactoring and code quality improvements
  • Score reflects clean refactoring (var to const, constant extraction), improved variant normalization, and better error handling. The type inconsistency with the .errors property is a minor concern but doesn't affect runtime safety
  • No files require special attention - the changes are straightforward refactoring improvements

Important Files Changed

Filename Overview
lib/google-font.js Refactored with var-to-const conversion, extracted DEFAULT_FORMAT constant, improved variant normalization (normal/normalitalic aliases), and collected errors instead of silently swallowing them in saveAtAsync

Sequence Diagram

sequenceDiagram
    participant Client
    participant GoogleFont
    participant _getFileMapAsync
    participant _normalizeVariant
    participant systemFont

    Client->>GoogleFont: saveAtAsync(variants, destFolder, format)
    GoogleFont->>GoogleFont: format = format || DEFAULT_FORMAT
    GoogleFont->>_getFileMapAsync: _getFileMapAsync(format)
    _getFileMapAsync-->>GoogleFont: fileList (variant->URL map)
    
    loop For each requested variant
        GoogleFont->>_normalizeVariant: _normalizeVariant(variant)
        Note over _normalizeVariant: Handles '400'/'normal' -> 'regular'<br/>'400italic'/'normalitalic' -> 'italic'
        _normalizeVariant-->>GoogleFont: normalized variant
        
        alt Variant URL exists in fileList
            GoogleFont->>systemFont: saveAt(url, dest, fileName)
            alt Success
                systemFont-->>GoogleFont: file path
                GoogleFont->>GoogleFont: Add to resultList
            else Error
                systemFont-->>GoogleFont: throw error
                GoogleFont->>GoogleFont: Collect error in errors array
            end
        end
    end
    
    alt errors.length > 0
        GoogleFont->>GoogleFont: Attach errors to resultList.errors
    end
    
    GoogleFont-->>Client: resultList (with errors if any)
Loading

@tinsever tinsever self-assigned this Jan 1, 2026
This was linked to issues Jan 1, 2026
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +248 to +251
// Attach errors to result for caller inspection if needed
if (errors.length > 0) {
/** @type {any} */ (resultList).errors = errors;
}
Copy link

Choose a reason for hiding this comment

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

logic: attaching errors as a property on an array changes the return type - callers expecting Promise<FontResult[]> may not handle the .errors property

Suggested change
// Attach errors to result for caller inspection if needed
if (errors.length > 0) {
/** @type {any} */ (resultList).errors = errors;
}
// Return both results and errors explicitly
return { results: resultList, errors: errors.length > 0 ? errors : undefined };
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/google-font.js
Line: 248:251

Comment:
**logic:** attaching errors as a property on an array changes the return type - callers expecting `Promise<FontResult[]>` may not handle the `.errors` property

```suggestion
	// Return both results and errors explicitly
	return { results: resultList, errors: errors.length > 0 ? errors : undefined };
```

How can I resolve this? If you propose a fix, please make it concise.

@tinsever tinsever merged commit 3835296 into v3-dev Jan 1, 2026
@tinsever tinsever deleted the fix/google-font-cleanup 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.

Font cache Request failureproofing Migrate to typescript

1 participant