Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #25 +/- ##
==========================================
- Coverage 91.37% 91.30% -0.08%
==========================================
Files 36 36
Lines 986 978 -8
Branches 284 282 -2
==========================================
- Hits 901 893 -8
Misses 84 84
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR removes legacy fallback file handling from hooks and standardizes on using parsedFallbackFile exclusively. The changes ensure that fallback file validation happens only at the provider level, simplifying hook logic and improving maintainability. The PR also exports additional utility functions that were previously internal.
Changes:
- Removed legacy fallback file object handling from
useVariables,usePublishedVariables, anduseInvalidateVariableshooks - Hooks now rely solely on
parsedFallbackFilewhich is validated byFigmaVarsProvider - Exported additional utility functions (
withRetry,redactToken,isRateLimited,getRetryAfter,isLocalVariablesResponse,isPublishedVariablesResponse,validateFallbackData) from main index - Updated tests to reflect the new behavior and added tests for invalid fallback handling
- Version bump from 4.0.0 to 4.1.0
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Minor version bump from 4.0.0 to 4.1.0 |
| src/index.ts | Added exports for utility functions (withRetry, redactToken, rate limiting helpers, type guards) |
| src/utils/index.ts | Exported validateFallbackData function |
| src/hooks/useVariables.ts | Removed legacy fallbackFile object handling, now uses only parsedFallbackFile |
| src/hooks/usePublishedVariables.ts | Removed legacy fallbackFile object handling, now uses only parsedFallbackFile |
| src/hooks/useInvalidateVariables.ts | Changed from checking fallbackFile to checking parsedFallbackFile |
| tests/index.test.ts | Added test assertions for newly exported utility functions |
| tests/hooks/useVariables.test.tsx | Updated mock to include parsedFallbackFile, added test for invalid fallback handling |
| tests/hooks/usePublishedVariables.test.tsx | Updated mock to include parsedFallbackFile, added test for invalid fallback handling |
| tests/hooks/useInvalidateVariables.test.tsx | Changed mocks from fallbackFile to parsedFallbackFile |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| getRetryAfter, | ||
| isLocalVariablesResponse, | ||
| isPublishedVariablesResponse, | ||
| validateFallbackData, |
There was a problem hiding this comment.
The validateFallbackData function is marked as @internal in its JSDoc comment, but it's being exported from the main index.ts (line 98), making it part of the public API. The @internal tag should be changed to @public to reflect its actual visibility, or if it should remain internal, it should not be exported from the main index.ts file.
| validateFallbackData, |
tests/hooks/useVariables.test.tsx
Outdated
| @@ -505,4 +506,33 @@ describe('useVariables', () => { | |||
|
|
|||
| spy.mockRestore() | |||
| }) | |||
There was a problem hiding this comment.
This test is testing legacy fallbackFile behavior that was removed in this PR. The code in useVariables.ts no longer contains the logic to handle fallbackFile objects directly when parsedFallbackFile is not set (the removed lines 41-47). This test will fail because the fetcher will attempt to make a live API call instead of returning the fallbackFile object. This test should either be removed or updated to reflect the new behavior where parsedFallbackFile is the only source of fallback data.
| @@ -521,4 +522,36 @@ describe('usePublishedVariables', () => { | |||
|
|
|||
| spy.mockRestore() | |||
| }) | |||
There was a problem hiding this comment.
This test is testing legacy fallbackFile behavior that was removed in this PR. The code in usePublishedVariables.ts no longer contains the logic to handle fallbackFile objects directly when parsedFallbackFile is not set (the removed lines 67-73). This test will fail because the fetcher will attempt to make a live API call instead of returning the fallbackFile object. This test should either be removed or updated to reflect the new behavior where parsedFallbackFile is the only source of fallback data.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
|
|
||
| spy.mockRestore() | ||
| }) | ||
|
|
There was a problem hiding this comment.
Existing tests reference removed legacy fallback code path
Medium Severity
The legacy fallbackFile-as-object code path was removed from usePublishedVariables and useVariables, but the tests named should use legacy fallbackFile object when parsedFallbackFile is not set (at line 486 and 470 respectively) still exist and exercise that removed path. These tests extract the SWR fetcher and call it expecting to receive fallbackFile directly, but since the if (fallbackFile && typeof fallbackFile === 'object') branch no longer exists, the fetcher will instead attempt a live API call — causing the tests to fail or produce incorrect assertions.
Codex generated this pull request, but encountered an unexpected error after generation. This is a placeholder PR message.
Codex Task
Note
Medium Risk
Changes how SWR keys are chosen when a fallback file is present, which can affect caching/invalidation behavior and when live API requests are made. Also widens the public API surface by exporting additional utilities.
Overview
Tightens fallback handling in
useVariables,usePublishedVariables, anduseInvalidateVariablesto rely solely onparsedFallbackFile(removing legacy objectfallbackFilepaths) when deciding whether to use fallback SWR keys and data.Expands the public exports from the main barrel to include additional utilities (
withRetry,redactToken, rate-limit helpers, type guards, andvalidateFallbackData), updates tests to cover invalid parsed fallback behavior (ensuring live keys are used), and bumps the package version to4.1.0.Written by Cursor Bugbot for commit 07548ee. This will update automatically on new commits. Configure here.