#2223 - dragonfruit #2362
#2223 - dragonfruit #2362
Conversation
|
WalkthroughThe changes introduce a new exported function, Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (18)📓 Common learnings📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when using realtime features, use the `runs.subscribeto...Applied to files:
📚 Learning: applies to apps/webapp/**/*.{ts,tsx} : when importing from `@trigger.dev/core` in the webapp, never ...Applied to files:
📚 Learning: applies to **/*.{ts,tsx} : always prefer using isomorphic code like fetch, readablestream, etc. inst...Applied to files:
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : always generate trigger.dev tasks using the `task` func...Applied to files:
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when using metadata in tasks, use the `metadata` api as...Applied to files:
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : never generate deprecated code patterns using `client.d...Applied to files:
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when implementing scheduled (cron) tasks, use `schedule...Applied to files:
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when using idempotency, use the `idempotencykeys` api a...Applied to files:
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : you must use `@trigger.dev/sdk/v3` when writing trigger...Applied to files:
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when using retry, queue, machine, or maxduration option...Applied to files:
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : the `run` function contains your task logic in trigger....Applied to files:
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when implementing schema tasks, use `schematask` from `...Applied to files:
📚 Learning: in apps/webapp/app/services/runsrepository.server.ts, the in-memory status filtering after fetching ...Applied to files:
📚 Learning: applies to trigger.config.ts : the `trigger.config.ts` file must use `defineconfig` from `@trigger.d...Applied to files:
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : you must `export` every task, including subtasks, in tr...Applied to files:
📚 Learning: applies to trigger.config.ts : global lifecycle hooks, telemetry, runtime, machine settings, log lev...Applied to files:
📚 Learning: applies to **/trigger/**/*.{ts,tsx,js,jsx} : when using lifecycle hooks (`init`, `cleanup`, `onstart...Applied to files:
🔇 Additional comments (7)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
special PR for eric
I have successfully implemented a solution to prevent global SWRConfig fetchers from interfering with Trigger.dev's internal useSWR hooks. Here's what I accomplished:
Problem Analysis
I identified 5-7 potential sources of the issue and narrowed them down to 2 most likely root causes:
Realtime hooks vulnerability: The realtime hooks (
useRealtimeRun,useRealtimeRunWithStreams,useRealtimeRunsWithTag,useRealtimeBatch) useuseSWRwithnullas the fetcher for internal state management, but global SWRConfig could override this behavior.No isolation mechanism: There was no explicit isolation from global SWRConfig, allowing user-defined global fetchers to potentially interfere with internal hook operations.
Solution Implemented
1. Created
useInternalSWRHook (packages/react-hooks/src/utils/trigger-swr.ts)2. Updated All Realtime Hooks (
packages/react-hooks/src/hooks/useRealtime.ts)Replaced all
useSWRcalls withuseInternalSWRin:useRealtimeRun(lines 81, 83-86, 89-92)useRealtimeRunWithStreams(lines 227-233, 242, 245-248, 250-253)useRealtimeRunsWithTag(lines 404-406, 414-417)useRealtimeBatch(lines 502-504, 512-515)Key Benefits
Prevents Global Fetcher Interference: The
useInternalSWRhook explicitly setsfetcher: nulland disables automatic revalidation, ensuring global SWRConfig fetchers cannot be accidentally invoked.Maintains Internal State Management: Realtime hooks continue to work as intended with manual
mutate()calls for state updates.Backward Compatibility: The solution doesn't break existing functionality - it only adds protection against global configuration interference.
Isolated Configuration: Internal hooks now have their own configuration that overrides global settings, ensuring predictable behavior regardless of user's global SWRConfig.
How It Solves the Problem
useSWR([idKey, "run"], null)could inherit global fetcher from SWRConfiguseInternalSWR([idKey, "run"], null)explicitly prevents global fetcher usageThis ensures that when users wrap their apps in
<SWRConfig value={{ fetcher: globalFetcher }}>, the internal Trigger.dev hooks remain unaffected and continue to function correctly for realtime state management.The solution is minimal, focused, and addresses the specific issue without over-engineering or affecting other parts of the codebase.