Skip to content

chore(core): rename req to request in onAuth#1144

Closed
NathanFlurry wants to merge 1 commit into07-28-fix_core_fix_request_not_being_passed_to_some_lifecycle_hooksfrom
07-28-chore_core_rename_req_to_request_in_onauth
Closed

chore(core): rename req to request in onAuth#1144
NathanFlurry wants to merge 1 commit into07-28-fix_core_fix_request_not_being_passed_to_some_lifecycle_hooksfrom
07-28-chore_core_rename_req_to_request_in_onauth

Conversation

@NathanFlurry
Copy link
Copy Markdown
Member

No description provided.

Copy link
Copy Markdown
Member Author

NathanFlurry commented Jul 28, 2025

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.

@claude
Copy link
Copy Markdown

claude bot commented Jul 28, 2025

Claude finished @NathanFlurry's task —— View job


PR Review: chore(core): rename req to request in onAuth

Review Checklist:

  • Read and analyze all changed files
  • Evaluate code quality and best practices
  • Check for potential bugs or issues
  • Assess performance considerations
  • Review security concerns
  • Examine test coverage
  • Provide comprehensive feedback

Overall Assessment: APPROVED

This is a well-executed refactoring that improves code consistency by standardizing parameter naming across the codebase.

Code Quality & Best Practices

✅ Excellent

  • Parameter names now match the TypeScript interface definitions in config.ts:231 (request: Request)
  • Consistent naming convention applied across all lifecycle hooks
  • Follows the established pattern used in type definitions

Completeness Check

✅ Comprehensive

  • All references to the old req parameter have been properly updated
  • Both destructured ({ request }) and direct parameter usage updated
  • Tests properly updated to reflect the changes
  • No remaining instances found in codebase search

Potential Issues

✅ No Issues Found

  • Changes are purely cosmetic parameter renames
  • All function signatures remain compatible
  • TypeScript types already expected request parameter name
  • No breaking changes to public APIs

Performance Considerations

✅ No Impact

  • Parameter renaming has zero performance impact
  • No changes to functionality or execution paths

Security Concerns

✅ No Security Impact

  • Changes are limited to parameter naming only
  • No modifications to authentication logic or request handling
  • Request object access patterns unchanged

Test Coverage

✅ Well Maintained

  • All affected test files updated appropriately (packages/core/src/driver-test-suite/tests/request-access.ts)
  • Test fixtures updated to match new parameter names
  • No test functionality compromised

Files Successfully Updated:

  • packages/core/fixtures/driver-test-suite/auth.ts:7,32 - onAuth parameter destructuring
  • packages/core/fixtures/driver-test-suite/request-access-auth.ts:9 - onAuth parameter name
  • packages/core/fixtures/driver-test-suite/request-access.ts:39,63 - createConnState and onBeforeConnect
  • packages/core/src/driver-test-suite/tests/request-access.ts - Test implementations
  • packages/core/src/manager/auth.ts:75 - authenticateRequest function call

This refactoring aligns implementation with type definitions and improves code readability. The changes are safe, complete, and maintain full backward compatibility.


@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Jul 28, 2025

Open in StackBlitz

@rivetkit/actor

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/actor@1144

@rivetkit/core

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/core@1144

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/db@1144

rivetkit

pnpm add https://pkg.pr.new/rivet-gg/rivetkit@1144

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/cloudflare-workers@1144

@rivetkit/redis

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/redis@1144

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/framework-base@1144

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/react@1144

commit: c073aa7

@graphite-app
Copy link
Copy Markdown

graphite-app bot commented Jul 28, 2025

Merge activity

  • Jul 28, 10:00 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
  • Jul 28, 10:01 PM UTC: CI is running for this pull request on a draft pull request (#1145) due to your merge queue CI optimization settings.
  • Jul 28, 10:02 PM UTC: Merged by the Graphite merge queue via draft PR: #1145.

@graphite-app graphite-app bot closed this Jul 28, 2025
@graphite-app graphite-app bot deleted the 07-28-chore_core_rename_req_to_request_in_onauth branch July 28, 2025 22:02
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.

1 participant