Skip to content

Codex-generated pull request #25

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marklearst merged 2 commits into from
Feb 23, 2026
Merged

Codex-generated pull request #25

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
07548ee
fix exports and fallback key selection edge case
marklearst Feb 17, 2026
c557cea
fix review comments on fallback tests and public docs
marklearst Feb 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@figma-vars/hooks",
"version": "4.0.0",
"version": "4.1.0",
"description": "Typed React hooks for managing Figma Variables, modes, tokens, and bindings via API.",
"author": "Mark Learst",
"license": "MIT",
Expand Down
5 changes: 3 additions & 2 deletions src/hooks/useInvalidateVariables.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@ import { getInvalidationKeys } from 'utils/swrKeys'
*/
export const useInvalidateVariables = () => {
const { mutate } = useSWRConfig()
const { token, fileKey, fallbackFile, providerId } = useFigmaTokenContext()
const { token, fileKey, parsedFallbackFile, providerId } =
useFigmaTokenContext()

const hasFallback = Boolean(fallbackFile)
const hasFallback = Boolean(parsedFallbackFile)

/**
* Invalidates all variable-related SWR cache entries.
Expand Down
18 changes: 3 additions & 15 deletions src/hooks/usePublishedVariables.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,10 @@ import { getPublishedVariablesKey } from 'utils/swrKeys'
* ```
*/
export const usePublishedVariables = () => {
const {
token,
fileKey,
fallbackFile,
parsedFallbackFile,
providerId,
swrConfig,
} = useFigmaTokenContext()
const { token, fileKey, parsedFallbackFile, providerId, swrConfig } =
useFigmaTokenContext()

const hasFallback = Boolean(fallbackFile || parsedFallbackFile)
const hasFallback = Boolean(parsedFallbackFile)

const key = getPublishedVariablesKey({
fileKey,
Expand All @@ -70,12 +64,6 @@ export const usePublishedVariables = () => {
return parsedFallbackFile as PublishedVariablesResponse
}

// Legacy support: if fallbackFile is an object but parsedFallbackFile wasn't set
// This can happen if the provider didn't validate the structure correctly
if (fallbackFile && typeof fallbackFile === 'object') {
return fallbackFile as PublishedVariablesResponse
}

const [u, t] = Array.isArray(args[0])
? args[0]
: ([args[0], args[1]] as const)
Expand Down
18 changes: 3 additions & 15 deletions src/hooks/useVariables.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,10 @@ import { getVariablesKey } from 'utils/swrKeys'
* @public
*/
export const useVariables = () => {
const {
token,
fileKey,
fallbackFile,
parsedFallbackFile,
providerId,
swrConfig,
} = useFigmaTokenContext()
const { token, fileKey, parsedFallbackFile, providerId, swrConfig } =
useFigmaTokenContext()

const hasFallback = Boolean(fallbackFile || parsedFallbackFile)
const hasFallback = Boolean(parsedFallbackFile)

const key = getVariablesKey({
fileKey,
Expand All @@ -44,12 +38,6 @@ export const useVariables = () => {
return parsedFallbackFile as LocalVariablesResponse
}

// Legacy support: if fallbackFile is an object but parsedFallbackFile wasn't set
// This can happen if the provider didn't validate the structure correctly
if (fallbackFile && typeof fallbackFile === 'object') {
return fallbackFile as LocalVariablesResponse
}

// At this point we expect live credentials; guard just in case
const [u, t] = Array.isArray(args[0])
? args[0]
Expand Down
7 changes: 7 additions & 0 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,17 @@ export {
*/
export {
filterVariables,
withRetry,
redactToken,
isFigmaApiError,
getErrorStatus,
getErrorMessage,
hasErrorStatus,
isRateLimited,
getRetryAfter,
isLocalVariablesResponse,
isPublishedVariablesResponse,
validateFallbackData,
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
validateFallbackData,

Copilot uses AI. Check for mistakes.
} from 'utils'

/**
Expand Down
1 change: 1 addition & 0 deletions src/utils/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export {
export {
isLocalVariablesResponse,
isPublishedVariablesResponse,
validateFallbackData,
} from 'utils/typeGuards'
export { redactToken } from 'utils/redactToken'
export type { RedactTokenOptions } from 'utils/redactToken'
Expand Down
4 changes: 2 additions & 2 deletions tests/hooks/useInvalidateVariables.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('useInvalidateVariables', () => {
token: 'test-token',
fileKey: 'test-file-key',
providerId: 'test-provider-id',
fallbackFile: '{}',
parsedFallbackFile: {},
})

const { result } = renderHook(() => useInvalidateVariables())
Expand Down Expand Up @@ -152,7 +152,7 @@ describe('useInvalidateVariables', () => {
token: 'test-token',
fileKey: 'test-file-key',
providerId: 'test-provider-id',
fallbackFile: '{}',
parsedFallbackFile: {},
})

const { result } = renderHook(() => useInvalidateVariables())
Expand Down
33 changes: 33 additions & 0 deletions tests/hooks/usePublishedVariables.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ describe('usePublishedVariables', () => {
token: null,
fileKey: null,
fallbackFile: mockPublishedVariablesResponse,
parsedFallbackFile: mockPublishedVariablesResponse,
} as ReturnType<typeof useFigmaTokenContextModule.useFigmaTokenContext>)

mockedUseSWR.mockReturnValue({
Expand Down Expand Up @@ -521,4 +522,36 @@ describe('usePublishedVariables', () => {

spy.mockRestore()
})
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown

@cursor cursor Bot Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Additional Locations (1)

Fix in Cursor Fix in Web

it('should use live key when fallbackFile exists but parsedFallbackFile is invalid', () => {
const spy = vi
.spyOn(useFigmaTokenContextModule, 'useFigmaTokenContext')
.mockReturnValue({
token: 'test-token',
fileKey: 'test-key',
fallbackFile: '{invalid-json}',
parsedFallbackFile: undefined,
providerId: 'test-provider',
} as ReturnType<typeof useFigmaTokenContextModule.useFigmaTokenContext>)

mockedUseSWR.mockReturnValue({
data: undefined,
error: undefined,
isLoading: false,
isValidating: false,
})

renderHook(() => usePublishedVariables())

expect(mockedUseSWR).toHaveBeenCalledWith(
[
'https://api.figma.com/v1/files/test-key/variables/published',
'test-token',
],
expect.any(Function),
undefined
)

spy.mockRestore()
})
})
30 changes: 30 additions & 0 deletions tests/hooks/useVariables.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ describe('useVariables', () => {
token: null,
fileKey: null,
fallbackFile: mockLocalVariablesResponse,
parsedFallbackFile: mockLocalVariablesResponse,
} as ReturnType<typeof useFigmaTokenContextModule.useFigmaTokenContext>)

mockedUseSWR.mockReturnValue({
Expand Down Expand Up @@ -505,4 +506,33 @@ describe('useVariables', () => {

spy.mockRestore()
})
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

it('should use live key when fallbackFile exists but parsedFallbackFile is invalid', () => {
const spy = vi
.spyOn(useFigmaTokenContextModule, 'useFigmaTokenContext')
.mockReturnValue({
token: 'test-token',
fileKey: 'test-key',
fallbackFile: '{invalid-json}',
parsedFallbackFile: undefined,
providerId: 'test-provider',
} as ReturnType<typeof useFigmaTokenContextModule.useFigmaTokenContext>)

mockedUseSWR.mockReturnValue({
data: undefined,
error: undefined,
isLoading: false,
isValidating: false,
})

renderHook(() => useVariables())

expect(mockedUseSWR).toHaveBeenCalledWith(
['https://api.figma.com/v1/files/test-key/variables/local', 'test-token'],
expect.any(Function),
undefined
)

spy.mockRestore()
})
})
7 changes: 7 additions & 0 deletions tests/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ describe('main index barrel file', () => {
// Check utils export
expect(indexModule.filterVariables).toBeDefined()
expect(typeof indexModule.filterVariables).toBe('function')
expect(indexModule.withRetry).toBeDefined()
expect(indexModule.redactToken).toBeDefined()
expect(indexModule.isRateLimited).toBeDefined()
expect(indexModule.getRetryAfter).toBeDefined()
expect(indexModule.isLocalVariablesResponse).toBeDefined()
expect(indexModule.isPublishedVariablesResponse).toBeDefined()
expect(indexModule.validateFallbackData).toBeDefined()

// Check that types are exported (they should be available as type exports)
// We can't directly test type exports, but we can check the module structure
Expand Down