Skip to content

Conversation

@jog1t
Copy link
Contributor

@jog1t jog1t commented Jan 26, 2026

TL;DR

Implemented a context caching system to maintain stable references across navigation in the frontend.

What changed?

  • Created a new cache.ts file with factory functions to create and cache data provider contexts
  • Added context getter functions to the router context
  • Modified routes to use the cached contexts instead of creating new ones on each navigation
  • Removed unused import of CancelledError from react-query
  • Enhanced context inheritance to properly pass parent context properties to child contexts
  • Added redirect handling for inspector URLs

How to test?

  1. Navigate between different parts of the application (organizations, projects, namespaces)
  2. Verify that navigation is smooth and state is preserved when returning to previously visited routes
  3. Check that inspector URLs are properly redirected when needed
  4. Ensure all data provider functionality works as expected with the cached contexts

Why make this change?

This change improves application performance and user experience by preventing unnecessary recreation of data provider contexts during navigation. By maintaining stable references to these contexts, we avoid redundant API calls and state resets when users navigate between routes. The caching system also ensures proper inheritance of context properties, making the code more maintainable and reducing potential bugs related to missing context data.

@jog1t jog1t marked this pull request as ready for review January 26, 2026 19:45
Copy link
Contributor Author

jog1t commented Jan 26, 2026


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.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 26, 2026

More templates

@rivetkit/cloudflare-workers

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@4041

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@4041

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@4041

commit: 2154cae

@claude
Copy link

claude bot commented Jan 26, 2026

Pull Request Review

This PR implements a context caching system to maintain stable references across navigation in the frontend.

Code Quality - Strengths

  1. Clean abstraction with well-organized cache layer
  2. Good TypeScript type safety
  3. Performance optimization preventing redundant API calls
  4. Proper context inheritance

Issues & Recommendations

1. Debug Console.log (CRITICAL)
Location: frontend/src/app/data-providers/cloud-data-provider.tsx:356
Remove before merging

2. Cache Key Generation Issue
Location: frontend/src/app/data-providers/cache.ts:46-48
Using .toString() on functions is unreliable for cache keys. Use explicit cache key parameter or WeakMap.

3. Cache Invalidation Missing
Module-scoped caches never cleared. Add invalidation on logout, TTL limits, and cleanup methods.

4. Potential Stale Data
Location: frontend/src/app/data-providers/cache.ts:77-86
Cached contexts may have stale parent data. Include parent in cache key validation.

5. Redirect Loop Risk
Location: frontend/src/routes/_context.tsx:92-113
getInspectorClientEndpoint could cause infinite redirect loop if URLs keep changing.

Performance

Positive: Prevents context recreation, reduces API calls
Concern: Unbounded cache growth with no memory cleanup

Security

Tokens stored in caches with no cleanup. Clear caches on logout to prevent token leakage.

Test Coverage

Missing tests for cache behavior, key generation, inheritance, invalidation, and redirect loops.

Action Items

Must Fix:

  • Remove console.log at cloud-data-provider.tsx:356

Should Fix:

  • Improve cache key generation for function-based tokens
  • Add cache invalidation on logout
  • Add basic unit tests
  • Add redirect loop protection

Nice to Have:

  • Implement LRU cache or size limits
  • Add JSDoc documentation

Overall Assessment

Solid performance improvement with clean TypeScript implementation. Address critical and important issues before merging.

@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 26, 2026

Graphite Automations

"Test" took an action on this PR • (01/26/26)

1 assignee was added to this PR based on Kacper Wojciechowski's automation.

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.

2 participants