Skip to content

Conversation

@NathanFlurry
Copy link
Member

No description provided.

@vercel
Copy link

vercel bot commented Nov 11, 2025

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

Project Deployment Preview Comments Updated (UTC)
rivetkit-serverless Ready Ready Preview Comment Nov 13, 2025 9:41pm
3 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
rivet-cloud Ignored Ignored Nov 13, 2025 9:41pm
rivet-inspector Ignored Ignored Preview Nov 13, 2025 9:41pm
rivet-site Ignored Ignored Preview Nov 13, 2025 9:41pm

This was referenced Nov 11, 2025
Copy link
Member Author

NathanFlurry commented Nov 11, 2025


How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

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 11-10-chore_rivetkit_remove_deprecated_packages branch from ecae19f to aaf96c9 Compare November 13, 2025 20:53
@NathanFlurry NathanFlurry force-pushed the 11-10-chore_rivetkit_add_support_for_conn_params_on_onrequest_and_onwebsocket branch from 8cf9a31 to 7d2f44d Compare November 13, 2025 20:53
@claude
Copy link

claude bot commented Nov 13, 2025

PR Review: Remove Deprecated Packages

Summary

This PR removes two deprecated RivetKit packages (@rivetkit/actor and @rivetkit/core) that were placeholders directing users to the unified rivetkit package. The cleanup is straightforward and well-executed.

Positive Findings

Clean Removal

  • All package files properly removed (13 files total)
  • Package dependencies cleaned from pnpm-lock.yaml
  • No remaining dependencies in package.json files across the workspace

Appropriate Scope

  • This is a pure cleanup operation with minimal risk
  • The packages being removed were already deprecated placeholders that threw errors on import
  • Net deletion of 144 lines vs 14 additions shows good code reduction

Issues Found

Documentation References Not Updated
The documentation still references the deprecated packages and needs to be updated:

File: website/src/content/docs/actors/fetch-and-websocket-handler.mdx

Lines 19, 52, 144, 169, 296, 333 contain references to:

  • @rivetkit/core (should be rivetkit)
  • @rivetkit/core/client (should be rivetkit/client)

Example from line 19:

import { type ActorContext, actor } from "@rivetkit/core";

Should be:

import { type ActorContext, actor } from "rivetkit";

File: website/src/data/examples/examples.ts

Line 28 contains an embedded package-lock.json example that may reference old package names (difficult to verify from diff, but worth checking).

Recommendations

  1. Required: Update the documentation file to use rivetkit instead of @rivetkit/core
  2. Verify: Check if the example package-lock.json in examples.ts needs updating
  3. Consider: Running a full-text search across the repository for any remaining references:
    grep -r "@rivetkit/\(actor\|core\)" --include="*.md" --include="*.mdx" --include="*.ts" --include="*.tsx"

Code Quality Assessment

  • Structure: Clean and complete removal
  • Dependencies: Properly handled in pnpm-lock.yaml
  • Conventions: Follows repository patterns (commit message follows conventional commits)
  • Risk Level: Low (only affects deprecated packages)

Verdict

The code changes are good, but the PR is incomplete due to the documentation references. Once the documentation is updated to use the new package names, this will be ready to merge.

The pnpm-lock.yaml changes look like standard dependency resolution updates and don't raise any concerns.

@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 13, 2025

Merge activity

@NathanFlurry NathanFlurry force-pushed the 11-10-chore_rivetkit_remove_deprecated_packages branch from aaf96c9 to 96c15e2 Compare November 13, 2025 21:39
@NathanFlurry NathanFlurry force-pushed the 11-10-chore_rivetkit_add_support_for_conn_params_on_onrequest_and_onwebsocket branch from 7d2f44d to 26c9559 Compare November 13, 2025 21:39
@claude
Copy link

claude bot commented Nov 13, 2025

PR Review: Remove Deprecated RivetKit Packages

Overview

This PR removes the deprecated @rivetkit/actor and @rivetkit/core placeholder packages. These were migration shims that threw errors directing users to use the unified rivetkit package instead.

✅ Positives

  1. Clean Removal: The package deletions are complete and thorough - all package files, configs, and build artifacts are properly removed
  2. Workspace Configuration: The pnpm-workspace.yaml uses a wildcard pattern (rivetkit-typescript/packages/*), so it naturally excludes the removed packages without needing updates
  3. Lock File Updates: The pnpm-lock.yaml has been properly updated to reflect the removed packages and their transitive dependencies
  4. Appropriate Timing: Since these packages were placeholders that threw errors, removing them is safe - anyone still importing them was already getting an error

⚠️ Issues Found

Critical: Documentation References Still Exist

The documentation still contains multiple references to the deprecated packages that must be updated:

File: website/src/content/docs/actors/fetch-and-websocket-handler.mdx

Lines 19, 52, 144, 169, 296, and 333 contain imports from @rivetkit/core:

import { type ActorContext, actor } from "@rivetkit/core";
import { createClient } from "@rivetkit/core/client";

These should be updated to:

import { type ActorContext, actor } from "rivetkit";
import { createClient } from "rivetkit/client";

File: website/src/data/examples/examples.ts

Line 28 contains a package-lock.json snippet that includes references to the deprecated packages. While this is example data, it should be updated to reflect current best practices.

Recommendations

  1. Update Documentation: Before merging, update all documentation examples to use the current rivetkit package name
  2. Search for Additional References: Run a comprehensive search for any other references:
    grep -r "@rivetkit/actor" --include="*.{md,mdx,ts,tsx,json}"
    grep -r "@rivetkit/core" --include="*.{md,mdx,ts,tsx,json}"
  3. Consider a Migration Note: If there's a changelog or migration guide, mention that these placeholder packages have been removed

Code Quality Assessment

  • Code Style: N/A (deletion only)
  • Best Practices: ✅ Follows proper package removal process
  • Security: ✅ No security concerns
  • Performance: ✅ Reduces package overhead
  • Test Coverage: ✅ No test changes needed (these were placeholder packages)

Summary

This is a good cleanup PR, but it should not be merged until the documentation references are updated. The outdated examples will confuse users and may cause them to use deprecated import paths.

Action Items Before Merge:

  1. Update all imports in website/src/content/docs/actors/fetch-and-websocket-handler.mdx from @rivetkit/core to rivetkit
  2. Update or verify the examples in website/src/data/examples/examples.ts
  3. Run a final check for any remaining references

Once documentation is updated, this PR will be ready to merge. 👍

@claude
Copy link

claude bot commented Nov 13, 2025

Pull Request Review

Summary

This PR removes two deprecated placeholder packages (@rivetkit/actor and @rivetkit/core) that have been superseded by the unified rivetkit package. The packages being removed were intentionally designed to throw errors when imported, directing users to migrate to the new package.

Code Quality and Best Practices ✅

Strengths:

  • Clean removal of deprecated code that was only serving as a migration placeholder
  • Properly removes all associated files (package.json, tsconfig.json, tsup.config.ts, turbo.json, README, and source files)
  • pnpm-lock.yaml correctly updated with dependency graph changes

Conventions Followed:

  • Conventional commit message format used: chore(rivetkit): remove deprecated packages
  • Follows the repository's cleanup and maintenance practices

Potential Issues ⚠️

Documentation Still References Deprecated Packages

The documentation files still contain references to the deprecated packages:

  1. website/src/content/docs/actors/fetch-and-websocket-handler.mdx (lines 19, 52, 144, 169, 296, 333):

    • Uses import { ... } from "@rivetkit/core"
    • Uses import { createClient } from "@rivetkit/core/client"
    • Should be updated to use rivetkit and rivetkit/client instead
  2. website/src/data/examples/examples.ts:

    • Contains example code/package-lock references to deprecated packages
    • Should be updated to reflect the correct package names

Recommendation: Update all documentation to use the new rivetkit package name before removing the deprecated packages. Otherwise, users following the documentation will encounter import errors.

Suggested Fix:

// Old (deprecated)
import { type ActorContext, actor } from "@rivetkit/core";
import { createClient } from "@rivetkit/core/client";

// New (correct)
import { type ActorContext, actor } from "rivetkit";
import { createClient } from "rivetkit/client";

Performance Considerations ✅

  • Removing unused packages reduces the overall monorepo size
  • Simplifies dependency resolution in pnpm workspace
  • No runtime performance impact as these packages were never used in production code

Security Concerns ✅

No security concerns. The removed packages were:

  • Only throwing errors when imported
  • Not used in any production code paths
  • Not exposing any functionality or data

Test Coverage ⚠️

Missing Validation:

  • No tests verify that the packages are actually removed
  • No validation that documentation examples still work after package removal

Recommendation: Before merging, verify that:

  1. All example code in documentation is updated and tested
  2. No other references to these packages exist in external documentation or example repositories
  3. The packages will be deprecated on npm with appropriate version ranges (if published)

NPM Package Lifecycle Consideration 📦

Important: If @rivetkit/actor and @rivetkit/core were published to npm, consider:

  1. Publishing a final version that throws a deprecation error (which these already do)
  2. Using npm deprecate command: npm deprecate @rivetkit/actor "*" "Use rivetkit instead"
  3. Adding a deprecation notice in the package.json or keeping the packages published with clear migration instructions

This ensures users who have not migrated get clear guidance.

Verdict

Status: Needs Changes 🔶

The package removal itself is clean and correct, but the documentation must be updated first to avoid breaking the developer experience. Once documentation is updated with the correct import paths, this PR will be ready to merge.

Action Items:

  1. ✏️ Update all documentation files to use rivetkit instead of @rivetkit/core and @rivetkit/actor
  2. ✏️ Update example code in website/src/data/examples/examples.ts
  3. ✅ Consider npm deprecation strategy (if packages were published)
  4. ✅ Verify no external documentation/repos reference these packages

Review generated by Claude Code

Base automatically changed from 11-10-chore_rivetkit_add_support_for_conn_params_on_onrequest_and_onwebsocket to main November 13, 2025 22:25
@NathanFlurry NathanFlurry merged commit c487a7e into main Nov 13, 2025
18 of 23 checks passed
@NathanFlurry NathanFlurry deleted the 11-10-chore_rivetkit_remove_deprecated_packages branch November 13, 2025 22:25
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.

3 participants