feat(config, react): Remove first class data loader cache #661
Conversation
🦋 Changeset detectedLatest commit: ff59bb4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Deploying stackflow-demo with
|
| Latest commit: |
ff59bb4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://edbcd47a.stackflow-demo.pages.dev |
| Branch Preview URL: | https://loader-cache-as-opt-in-featu.stackflow-demo.pages.dev |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
stackflow-docs | ff59bb4 | Commit Preview URL | Dec 15 2025, 07:21 AM |
📝 WalkthroughSummary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR removes the data loader cache mechanism from the Stackflow library. It eliminates the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
config/src/ActivityLoader.ts (2)
11-11: Consider returningloaderFndirectly.The wrapper function
(args: ActivityLoaderArgs<ActivityName>) => loaderFn(args)is unnecessary sinceloaderFnalready matches theActivityLoader<ActivityName>signature.Apply this diff to simplify:
- return (args: ActivityLoaderArgs<ActivityName>) => loaderFn(args); + return loaderFn;
4-6: Consider using a generic type parameter for the return type.The
anyreturn type reduces type safety. Consider adding a generic type parameter to improve type inference for consumers.Example refactor:
-export type ActivityLoader<ActivityName extends RegisteredActivityName> = ( - args: ActivityLoaderArgs<ActivityName>, -) => any; +export type ActivityLoader< + ActivityName extends RegisteredActivityName, + TReturn = any, +> = (args: ActivityLoaderArgs<ActivityName>) => TReturn; -export function loader<ActivityName extends RegisteredActivityName>( - loaderFn: (args: ActivityLoaderArgs<ActivityName>) => any, -): ActivityLoader<ActivityName> { +export function loader< + ActivityName extends RegisteredActivityName, + TReturn = any, +>( + loaderFn: (args: ActivityLoaderArgs<ActivityName>) => TReturn, +): ActivityLoader<ActivityName, TReturn> { return (args: ActivityLoaderArgs<ActivityName>) => loaderFn(args); }As per coding guidelines, TypeScript should be written with strict typing enabled.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.changeset/real-cats-rest.md(1 hunks)config/src/ActivityLoader.ts(1 hunks)integrations/react/src/future/stackflow.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- integrations/react/src/future/stackflow.tsx
🧰 Additional context used
📓 Path-based instructions (3)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Include a Changeset entry for any user-facing package change
Files:
.changeset/real-cats-rest.md
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Write source in TypeScript with strict typing enabled across the codebase
Files:
config/src/ActivityLoader.ts
config/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
For the Future API, define activities and routes in @stackflow/config using defineConfig(), keeping component injection separate
Files:
config/src/ActivityLoader.ts
🧬 Code graph analysis (1)
config/src/ActivityLoader.ts (2)
config/src/RegisteredActivityName.ts (1)
RegisteredActivityName(3-6)config/src/ActivityLoaderArgs.ts (1)
ActivityLoaderArgs(6-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: stackflow-docs
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
.changeset/real-cats-rest.md (1)
1-6: LGTM! Changeset properly documents the breaking changes.The changeset correctly marks major version bumps for both affected packages (@stackflow/react and @stackflow/config) and provides a clear description of the change. The version bump types align with the breaking nature of removing first-class data loader cache functionality.
config/src/ActivityLoader.ts (1)
4-6: LGTM! Type definition correctly simplified.The removal of the
loaderCacheMaxAgeproperty and simplification to a plain function type aligns with the PR objective to remove first-class data loader cache functionality.
There was a problem hiding this comment.
I could not know why loadData function should be placed in stackflow.tsx. The operation of current loadData function is very plain, therefore this operation could be moved away but I could not ensure where this operation should belong to.
We should consider of this later. Approved
Data caching must be done in dedicated data fetching/synching layer such as relay and tanstack DB. We're not going to tackle this problem on our own.