fix(desktop): replace node-pty with @zenyr/bun-pty for Bun compatibility#525
Closed
andreasasprou wants to merge 1 commit intosuperset-sh:mainfrom
Closed
fix(desktop): replace node-pty with @zenyr/bun-pty for Bun compatibility#525andreasasprou wants to merge 1 commit intosuperset-sh:mainfrom
andreasasprou wants to merge 1 commit intosuperset-sh:mainfrom
Conversation
Resolves node-gyp build errors during setup by replacing node-pty with @zenyr/bun-pty, which is specifically designed for Bun environments. The error "Class extends value #<Object> is not a constructor or null" was caused by node-pty expecting Node.js runtime symbols not available in Bun.
Contributor
📝 WalkthroughWalkthroughUpdated the PTY library dependency in the desktop app package configuration, replacing Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
apps/desktop/package.json
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.021Z
Learning: Applies to apps/desktop/src/**/*.{ts,tsx} : Move Node.js functionality needed in Electron renderer to `src/main/lib/` and communicate via IPC
📚 Learning: 2025-12-28T01:56:39.021Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.021Z
Learning: Applies to apps/desktop/src/**/*.{ts,tsx} : Move Node.js functionality needed in Electron renderer to `src/main/lib/` and communicate via IPC
Applied to files:
apps/desktop/package.json
📚 Learning: 2025-12-28T01:56:39.021Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.021Z
Learning: Use Bun as the package manager for this monorepo, not npm/yarn/pnpm
Applied to files:
apps/desktop/package.json
| "@xterm/addon-web-links": "^0.11.0", | ||
| "@xterm/addon-webgl": "^0.18.0", | ||
| "@xterm/xterm": "^5.5.0", | ||
| "@zenyr/bun-pty": "^0.4.4", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all node-pty imports have been updated and check PTY usage location
echo "=== Checking for remaining node-pty imports ==="
rg -n --type=ts --type=tsx "from ['\"]node-pty['\"]|require\(['\"]node-pty['\"]\)" apps/desktop/src/ 2>/dev/null || echo "No results"
echo -e "\n=== Checking new @zenyr/bun-pty imports ==="
rg -n --type=ts --type=tsx "from ['\"]@zenyr/bun-pty['\"]|require\(['\"]@zenyr/bun-pty['\"]\)" apps/desktop/src/ 2>/dev/null || echo "No results"
echo -e "\n=== Checking PTY usage location (should be in main process, not renderer) ==="
rg -n --type=ts --type=tsx -C3 "@zenyr/bun-pty|pty\|IPty" apps/desktop/src/renderer/ 2>/dev/null || echo "No PTY usage in renderer"
echo -e "\n=== Checking for @types/node-pty in package.json ==="
rg -n "@types/node-pty" apps/desktop/package.json 2>/dev/null || echo "Not found"
echo -e "\n=== Checking PTY usage in main process ==="
rg -n --type=ts --type=tsx "@zenyr/bun-pty|pty\|IPty" apps/desktop/src/main/ 2>/dev/null || echo "No results"Repository: superset-sh/superset
Length of output: 401
🌐 Web query:
@zenyr/bun-pty node-pty drop-in replacement API compatibility
💡 Result:
Short answer: Mostly — @zenyr/bun-pty implements a node-pty–like API (spawn, write, resize, kill, onData/onExit, pid/cols/rows/process) and is intended as a Bun-native replacement, so common node-pty code will often work with little or no changes. [1][2]
Notes / caveats:
- Method names/shape: bun-pty exposes spawn and an IPty with onData/onExit, write, resize, kill and similar properties — matching the typical node-pty surface. [1][2]
- Event vs callback style: modern node-pty docs also show onData/onExit APIs (not only EventEmitter), but if your code relies on older event-emitter internals or undocumented behaviors you should test. [2]
- Options/edge cases: confirm options you use (env, cwd, name, cols/rows, encoding, signal handling) behave identically in your target Bun version; prebuilt binaries and Bun runtime differences may affect behavior. [1]
- Recommendation: treat it as “mostly drop-in” for typical terminal use, but run your test-suite (or quick smoke tests) for any advanced/edge-case features (signals, custom streams, platforms). If you want, tell me a specific node-pty code sample and I’ll point out required changes/test checklist.
Sources:
[1] @zenyr/bun-pty README (API, types, examples).
[2] node-pty (API / examples).
🏁 Script executed:
#!/bin/bash
# Check all file types for PTY usage
echo "=== Searching all file types for node-pty usage ==="
rg "node-pty" apps/desktop/ 2>/dev/null || echo "No node-pty found"
echo -e "\n=== Searching all file types for @zenyr/bun-pty usage ==="
rg "@zenyr/bun-pty" apps/desktop/ 2>/dev/null || echo "No @zenyr/bun-pty found"
echo -e "\n=== Checking if pty/IPty is used anywhere in codebase ==="
rg "spawn.*pty|IPty|bun-pty" apps/desktop/src/ 2>/dev/null || echo "No PTY usage found"
echo -e "\n=== List all files in apps/desktop/src/main/ ==="
find apps/desktop/src/main -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" \) 2>/dev/null | head -20Repository: superset-sh/superset
Length of output: 3099
Complete the migration from node-pty to @zenyr/bun-pty across the entire codebase.
The dependency has been added to package.json, but the integration is incomplete. The following files still import and use node-pty:
apps/desktop/src/main/lib/terminal/session.ts— imports"node-pty"apps/desktop/src/main/lib/terminal/types.ts— imports"node-pty"for typesapps/desktop/src/main/lib/terminal/manager.test.ts— imports and mocks"node-pty"
Additionally, the build and tooling configuration still reference node-pty:
apps/desktop/electron-builder.ts— copy rules fornode-ptynative modulesapps/desktop/electron.vite.config.ts— marksnode-ptyas externalapps/desktop/scripts/copy-native-modules.ts— listsnode-ptyas native module
Update all these files to import @zenyr/bun-pty instead. Verify that the @zenyr/bun-pty API is compatible with existing terminal operations (spawn, IPty interface, write, resize, kill, onData/onExit).
🤖 Prompt for AI Agents
In apps/desktop/package.json around line 68 and across the codebase, the
dependency was added but imports and tooling still reference node-pty; update
all occurrences to @zenyr/bun-pty: replace import "node-pty" with
"@zenyr/bun-pty" in apps/desktop/src/main/lib/terminal/session.ts and
apps/desktop/src/main/lib/terminal/types.ts (adjust type imports to use the
IPty/type names exported by @zenyr/bun-pty), update tests in
apps/desktop/src/main/lib/terminal/manager.test.ts to mock and import
@zenyr/bun-pty, and change build/tooling references in
apps/desktop/electron-builder.ts, apps/desktop/electron.vite.config.ts, and
apps/desktop/scripts/copy-native-modules.ts to reference @zenyr/bun-pty; while
making these changes, ensure API compatibility by mapping any differing method
or event names (spawn, write, resize, kill, onData/onExit or their equivalents)
to the existing terminal usage and update types or small adapters where
necessary so existing code calls continue to work.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
node-ptywith@zenyr/bun-ptywhich is designed for Bun compatibilityProblem
Running
./.superset/setup.shwith Bun fails with node-gyp errors becausenode-ptyexpects Node.js runtime symbols not available in Bun.Solution
Replace
node-ptywith@zenyr/bun-pty, a drop-in replacement specifically built for Bun environments that provides the same terminal API.Files Changed
apps/desktop/package.json: Updated dependencybun.lock: Updated lock file with new dependency versionsTesting
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.