Skip to content

chore(core): remove unused bufferutil #1337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: 10-02-fix_core_fix_json_encoding
Choose a base branch
from
Open

chore(core): remove unused bufferutil #1337

wants to merge 1 commit into from
+4 −6

Conversation

NathanFlurry
Copy link
Member

No description provided.

@vercel Vercel
Copy link

vercel bot commented Oct 3, 2025 •
edited
Loading

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

Project Deployment Preview Comments Updated (UTC)
rivetkit-serverless Ready Ready Preview Comment Oct 3, 2025 4:48am

@claude Claude
Copy link

claude bot commented Oct 3, 2025 •
edited
Loading

Claude finished @NathanFlurry's task —— View job

Code Review for bufferutil Removal

Reviewing the removal of unused bufferutil dependency from the core package.

Todo List:

  • Read the changed package.json file
  • Search codebase for any remaining bufferutil usage
  • Check for potential breaking changes
  • Verify impact on WebSocket functionality
  • Assess dependency cleanup completeness
  • Provide comprehensive review feedback

Review Summary

✅ Approved - This is a clean dependency removal with no functional impact.

Analysis

Code Quality & Best Practices

  • Excellent: This follows proper dependency hygiene by removing unused packages
  • Clean commit: Single-purpose commit with clear, descriptive message
  • Proper location: Removed from devDependencies in packages/rivetkit/package.json:191

Verification of Removal Safety

  • No imports found: Comprehensive search shows no import/require statements for bufferutil
  • No direct usage: No references to bufferutil in any TypeScript/JavaScript files
  • WebSocket implementation: Examined packages/rivetkit/src/common/websocket.ts:22 - uses ws package directly without bufferutil optimization

Impact Assessment

  • Zero breaking changes: bufferutil was only in devDependencies and unused
  • Performance: No performance impact as bufferutil was never utilized
  • Bundle size: Slight improvement due to reduced dependency tree
  • WebSocket functionality: Unaffected - the codebase uses ws package which can optionally use bufferutil for performance, but works fine without it

Security Considerations

  • No security concerns: Removing unused dependencies reduces attack surface
  • Dependency maintenance: One less package to monitor for vulnerabilities

Test Coverage

  • Low risk: Since bufferutil was completely unused, existing tests should remain valid
  • Note: Unable to run type checking due to permission restrictions, but no type-related issues expected

Recommendation

This PR should be approved and merged. It's a straightforward cleanup that:

  • Removes genuinely unused code
  • Reduces dependency footprint
  • Follows good maintenance practices
  • Has zero functional impact

The Vercel deployment error appears unrelated to this change and likely stems from other build issues.

@NathanFlurry NathanFlurry mentioned this pull request Oct 3, 2025
Open
@NathanFlurry Graphite App
Copy link
Member Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • merge queue - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@NathanFlurry NathanFlurry force-pushed the 10-02-chore_core_remove_unused_bufferutil branch from c4df809 to c92b691 Compare October 3, 2025 04:47
@claude Claude
Copy link

claude bot commented Oct 3, 2025 •
edited
Loading

Claude finished @NathanFlurry's task —— View job

Code Review for bufferutil Removal

Reviewing the removal of unused bufferutil dependency from the core package.

Todo List:

  • Read the repository guidelines (CLAUDE.md)
  • Read the changed package.json file
  • Search codebase for any remaining bufferutil usage
  • Check for potential breaking changes
  • Verify impact on WebSocket functionality
  • Assess dependency cleanup completeness
  • Provide comprehensive review feedback

Review Summary

✅ Approved - This is a clean dependency removal with no functional impact.

Analysis

Code Quality & Best Practices

  • Excellent: This follows proper dependency hygiene by removing unused packages
  • Clean commit: Single-purpose commit with clear, descriptive message
  • Proper location: Removed from devDependencies in packages/rivetkit/package.json:191

Verification of Removal Safety

  • No imports found: Comprehensive search shows no import/require statements for bufferutil
  • No direct usage: No references to bufferutil in any TypeScript/JavaScript files
  • WebSocket implementation: Examined packages/rivetkit/src/common/websocket.ts:22 - uses ws package directly without bufferutil optimization

Impact Assessment

  • Zero breaking changes: bufferutil was only in devDependencies and unused
  • Performance: No performance impact as bufferutil was never utilized
  • Bundle size: Slight improvement due to reduced dependency tree
  • WebSocket functionality: Unaffected - the codebase uses ws package which can optionally use bufferutil for performance, but works fine without it

Security Considerations

  • No security concerns: Removing unused dependencies reduces attack surface
  • Dependency maintenance: One less package to monitor for vulnerabilities

Test Coverage

  • Low risk: Since bufferutil was completely unused, existing tests should remain valid
  • Note: Unable to run type checking due to permission restrictions, but no type-related issues expected

Recommendation

This PR should be approved and merged. It's a straightforward cleanup that:

  • Removes genuinely unused code
  • Reduces dependency footprint
  • Follows good maintenance practices
  • Has zero functional impact

The Vercel deployment error appears unrelated to this change and likely stems from other build issues.

@pkg-pr-new pkg.pr.new
Copy link

pkg-pr-new bot commented Oct 3, 2025

More templates

@rivetkit/actor

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/actor@1337

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/cloudflare-workers@1337

@rivetkit/core

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/core@1337

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/db@1337

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/framework-base@1337

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/next-js@1337

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/react@1337

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivetkit@1337

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/sql-loader@1337

commit: c92b691

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@NathanFlurry