Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

fixed ts sdk publishing job to use later node version bc earlier versions of Node do not support File type

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Oct 13, 2025

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

Project Deployment Preview Comments Updated (UTC)
docs Building Building Preview Comment Oct 13, 2025 7:13pm

@waleedlatif1 waleedlatif1 merged commit 1773530 into staging Oct 13, 2025
3 of 4 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/sdks branch October 13, 2025 19:13
Copy link
Contributor

@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.

Greptile Overview

Summary

Updated the TypeScript SDK publishing workflow to use Node.js 22 instead of Node.js 18. This change is necessary because the SDK uses the native File API (line 133 in packages/ts-sdk/src/index.ts) for file upload functionality, which was introduced in Node.js 20.

Key Points:

  • The SDK's convertFilesToBase64 method relies on instanceof File checks and File.arrayBuffer() method
  • Node.js 18 does not have native File support, which would cause the publishing process to fail
  • Node.js 20+ includes the native File API as part of the Fetch API implementation

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk
  • The change is minimal and well-justified - upgrading from Node 18 to Node 22 to support the native File API used in the SDK. However, there's a minor concern: the package.json specifies engines.node: ">=16", which could mislead users into thinking Node 16-19 is supported when the SDK actually requires Node 20+ for the File API to work properly in Node environments
  • Consider updating packages/ts-sdk/package.json to reflect the actual Node.js version requirement (>=20)

Important Files Changed

File Analysis

Filename Score Overview
.github/workflows/publish-ts-sdk.yml 4/5 Updated Node.js version from 18 to 22 to support the File API used in the TypeScript SDK for file upload functionality

Sequence Diagram

sequenceDiagram
    participant GHA as GitHub Actions
    participant Node as Node.js 22
    participant Bun as Bun Runtime
    participant NPM as NPM Registry
    participant SDK as TypeScript SDK

    GHA->>GHA: Checkout repository
    GHA->>Bun: Setup Bun 1.2.22
    GHA->>Node: Setup Node.js 22
    Note over Node: Required for File API support
    GHA->>Bun: Install dependencies
    GHA->>SDK: Run tests
    SDK-->>GHA: Tests pass
    GHA->>SDK: Build package
    SDK->>SDK: Compile TypeScript (uses File API)
    SDK-->>GHA: Build artifacts
    GHA->>Node: Get package version
    Node-->>GHA: Return version
    GHA->>NPM: Check if version exists
    NPM-->>GHA: Version status
    alt Version does not exist
        GHA->>NPM: Publish package
        NPM-->>GHA: Published successfully
        GHA->>GHA: Create GitHub Release
    else Version exists
        GHA->>GHA: Skip publishing
    end
Loading

Additional Comments (1)

  1. packages/ts-sdk/package.json, line 46-48 (link)

    syntax: The engines.node field specifies >=16, but the SDK uses the native File API (in src/index.ts:133) which is only available in Node 20+. Update to reflect the actual requirement:

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

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