[PR #120] feat(sdk): make the SDK loadable in a browser #124

New issue

Open

opened 2026-10-31 22:04:47 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 22:04:47 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-typescript-sdk/pull/120
Author: @benjcooley
Created: 9/3/2026
Status: 🔄 Open

Base: main ← Head: ben/sdk-browser-safe

📝 Commits (1)

add6c32 feat(sdk): make the SDK loadable in a browser

📊 Changes

7 files changed (+109 additions, -16 deletions)

View changed files

➕ src/browser-safety.test.ts (+50 -0)
📝 src/sdk/assets.test.ts (+11 -1)
📝 src/sdk/assets.ts (+24 -2)
📝 src/sdk/core.ts (+1 -3)
📝 src/sdk/hashing.ts (+8 -3)
📝 src/sdk/outputs.ts (+8 -4)
📝 src/sdk/workflows.ts (+7 -3)

📄 Description

Summary

I'm making the SDK compatible with running in a web page. It previously only ran in Node.js. We need this because our Workshop pages call Router endpoints, and the page should use the same library the code snippet next to it teaches.

Five files imported Node-only tools at the top of the file. A browser build tool reads every import up front, before it knows which functions will actually run, so those got pulled in even when nothing called them. Browsers have no file system, so the build breaks — on main it fails with "randomUUID" is not exported by "__vite-browser-external:node:crypto".

Two of them didn't need Node at all. One generates a random ID, which browsers and Node both do natively. The other splits filenames apart, which is a few lines of string handling, now written inline.

Those inlined helpers match what node:path actually does rather than picking one set of rules. node:path dispatches on platform: on Windows a backslash is a path separator, on Linux and macOS it's an ordinary filename character. So a POSIX file genuinely named a\b.txt keeps its backslash exactly as before. A browser has no filesystem paths, so it takes the POSIX rules.

The other three really do read and write files. They behave the same, but now ask for their Node tools inside the function instead of at the top, so that only happens if you call them. A bundler puts those in separate chunks a browser never fetches. assets.ts already worked this way for openAsBlob; this extends it to the rest.

Provenance

Authored by: Claude Code, directed by @benjcooley
Verified: pnpm typecheck, pnpm test (409 passed, 24 files), pnpm lint, pnpm format:check, pnpm build. Also built a real Vite browser bundle against this branch and against main — see the test plan.
Deviations: none

Reviewer context

Type: compatibility fix. No behaviour change for Node callers, no API change.
Not in scope: models.list(). The router surface is hand-written rather than generated, so adding a catalog listing is a separate change with its own pagination design.

Test plan

Existing suite passes unchanged, plus one new test: 409 total
Built a Vite browser bundle importing the package. Against main it fails on the node:crypto import; against this branch it builds clean, 115 modules, and the main entry chunk contains no node: references. The four Node modules appear only as separate lazy chunks that are never fetched
browser-safety.test.ts walks the shipped source and fails on any top-of-file Node import, naming file and line. Checked against a negative control: reintroducing import { randomUUID } from "node:crypto" makes it fail with that exact location, and it passes again once reverted
assets.test.ts asserts the inlined helpers agree with node:path on this platform, including the backslash case that differs between win32 and posix
Not behind a feature flag. Nothing user-visible changes and the Node API is identical

Summary by CodeRabbit

Bug Fixes
- Improved browser compatibility by allowing SDK modules to load without immediately requiring Node.js built-ins.
- Preserved asset path handling across operating systems.
- Ensured idempotency keys continue to use UUIDv4 formatting and remain unique.
Documentation
- Clarified which file-based operations require Node.js and provided browser-compatible alternatives.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfy-typescript-sdk/pull/120 **Author:** [@benjcooley](https://github.com/benjcooley) **Created:** 9/3/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `ben/sdk-browser-safe` --- ### 📝 Commits (1) - [`add6c32`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/add6c327e3206c0e60fb44d9ae386e61fb131312) feat(sdk): make the SDK loadable in a browser ### 📊 Changes **7 files changed** (+109 additions, -16 deletions) <details> <summary>View changed files</summary> ➕ `src/browser-safety.test.ts` (+50 -0) 📝 `src/sdk/assets.test.ts` (+11 -1) 📝 `src/sdk/assets.ts` (+24 -2) 📝 `src/sdk/core.ts` (+1 -3) 📝 `src/sdk/hashing.ts` (+8 -3) 📝 `src/sdk/outputs.ts` (+8 -4) 📝 `src/sdk/workflows.ts` (+7 -3) </details> ### 📄 Description ## Summary I'm making the SDK compatible with running in a web page. It previously only ran in Node.js. We need this because our Workshop pages call Router endpoints, and the page should use the same library the code snippet next to it teaches. Five files imported Node-only tools at the top of the file. A browser build tool reads every import up front, before it knows which functions will actually run, so those got pulled in even when nothing called them. Browsers have no file system, so the build breaks — on `main` it fails with `"randomUUID" is not exported by "__vite-browser-external:node:crypto"`. Two of them didn't need Node at all. One generates a random ID, which browsers and Node both do natively. The other splits filenames apart, which is a few lines of string handling, now written inline. Those inlined helpers match what `node:path` actually does rather than picking one set of rules. `node:path` dispatches on platform: on Windows a backslash is a path separator, on Linux and macOS it's an ordinary filename character. So a POSIX file genuinely named `a\b.txt` keeps its backslash exactly as before. A browser has no filesystem paths, so it takes the POSIX rules. The other three really do read and write files. They behave the same, but now ask for their Node tools inside the function instead of at the top, so that only happens if you call them. A bundler puts those in separate chunks a browser never fetches. `assets.ts` already worked this way for `openAsBlob`; this extends it to the rest. ## Provenance Authored by: Claude Code, directed by @benjcooley Verified: `pnpm typecheck`, `pnpm test` (409 passed, 24 files), `pnpm lint`, `pnpm format:check`, `pnpm build`. Also built a real Vite browser bundle against this branch and against `main` — see the test plan. Deviations: none ## Reviewer context Type: compatibility fix. No behaviour change for Node callers, no API change. Not in scope: `models.list()`. The router surface is hand-written rather than generated, so adding a catalog listing is a separate change with its own pagination design. ## Test plan - [x] Existing suite passes unchanged, plus one new test: 409 total - [x] Built a Vite browser bundle importing the package. Against `main` it **fails** on the `node:crypto` import; against this branch it builds clean, 115 modules, and the main entry chunk contains no `node:` references. The four Node modules appear only as separate lazy chunks that are never fetched - [x] `browser-safety.test.ts` walks the shipped source and fails on any top-of-file Node import, naming file and line. Checked against a negative control: reintroducing `import { randomUUID } from "node:crypto"` makes it fail with that exact location, and it passes again once reverted - [x] `assets.test.ts` asserts the inlined helpers agree with `node:path` on this platform, including the backslash case that differs between win32 and posix - [ ] Not behind a feature flag. Nothing user-visible changes and the Node API is identical  ## Summary by CodeRabbit * **Bug Fixes** * Improved browser compatibility by allowing SDK modules to load without immediately requiring Node.js built-ins. * Preserved asset path handling across operating systems. * Ensured idempotency keys continue to use UUIDv4 formatting and remain unique. * **Documentation** * Clarified which file-based operations require Node.js and provided browser-compatible alternatives.  --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-31 22:04:47 +00:00

bsakaguchi referenced this issue

2026-10-31 22:04:49 +00:00

[PR #124] [MERGED] docs(changelog): stage 0.1.9 for the timeout fix #127