From d662d44e7702a8fc2224ddc03b83178a3dd6768b Mon Sep 17 00:00:00 2001 From: Jan Six Date: Wed, 4 Dec 2024 09:20:51 +0100 Subject: [PATCH 1/2] fix variable setting for various cases --- .../plugin/setBooleanValuesOnVariable.test.ts | 47 +++++++++++ .../src/plugin/setBooleanValuesOnVariable.ts | 15 ++-- .../plugin/setColorValuesOnVariable.test.ts | 78 +++++++++++++++++++ .../src/plugin/setColorValuesOnVariable.ts | 23 ++++-- .../plugin/setNumberValuesOnVariable.test.ts | 68 ++++++++++++++++ .../src/plugin/setNumberValuesOnVariable.ts | 13 +++- .../plugin/setStringValuesOnVariable.test.ts | 46 +++++++++++ .../src/plugin/setStringValuesOnVariable.ts | 8 +- .../src/utils/isAliasReference.ts | 3 + 9 files changed, 278 insertions(+), 23 deletions(-) create mode 100644 packages/tokens-studio-for-figma/src/plugin/setBooleanValuesOnVariable.test.ts create mode 100644 packages/tokens-studio-for-figma/src/plugin/setColorValuesOnVariable.test.ts create mode 100644 packages/tokens-studio-for-figma/src/plugin/setNumberValuesOnVariable.test.ts create mode 100644 packages/tokens-studio-for-figma/src/plugin/setStringValuesOnVariable.test.ts create mode 100644 packages/tokens-studio-for-figma/src/utils/isAliasReference.ts diff --git a/packages/tokens-studio-for-figma/src/plugin/setBooleanValuesOnVariable.test.ts b/packages/tokens-studio-for-figma/src/plugin/setBooleanValuesOnVariable.test.ts new file mode 100644 index 000000000..50f35d755 --- /dev/null +++ b/packages/tokens-studio-for-figma/src/plugin/setBooleanValuesOnVariable.test.ts @@ -0,0 +1,47 @@ +import setBooleanValuesOnVariable from './setBooleanValuesOnVariable'; +import { isVariableWithAliasReference } from '@/utils/isAliasReference'; + +jest.mock('@/utils/isAliasReference'); + +describe('setBooleanValuesOnVariable', () => { + let mockVariable: Variable; + const mockMode = 'light'; + + beforeEach(() => { + mockVariable = { + valuesByMode: { + light: false, + }, + setValueForMode: jest.fn(), + } as unknown as Variable; + + (isVariableWithAliasReference as unknown as jest.Mock).mockReset(); + }); + + it('should set new boolean value when different from existing value', () => { + (isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false); + setBooleanValuesOnVariable(mockVariable, mockMode, 'true'); + expect(mockVariable.setValueForMode).toHaveBeenCalledWith(mockMode, true); + }); + + it('should not set value when new value equals existing value', () => { + mockVariable.valuesByMode.light = false; + (isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false); + setBooleanValuesOnVariable(mockVariable, mockMode, 'false'); + expect(mockVariable.setValueForMode).not.toHaveBeenCalled(); + }); + + it('should handle alias references', () => { + mockVariable.valuesByMode.light = '{bool.primary}'; + (isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(true); + setBooleanValuesOnVariable(mockVariable, mockMode, 'true'); + expect(mockVariable.setValueForMode).toHaveBeenCalledWith(mockMode, true); + }); + + it('should return early if existing value is invalid', () => { + mockVariable.valuesByMode.light = 'invalid' as any; + (isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false); + setBooleanValuesOnVariable(mockVariable, mockMode, 'true'); + expect(mockVariable.setValueForMode).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/tokens-studio-for-figma/src/plugin/setBooleanValuesOnVariable.ts b/packages/tokens-studio-for-figma/src/plugin/setBooleanValuesOnVariable.ts index 5111d2790..11bdba925 100644 --- a/packages/tokens-studio-for-figma/src/plugin/setBooleanValuesOnVariable.ts +++ b/packages/tokens-studio-for-figma/src/plugin/setBooleanValuesOnVariable.ts @@ -1,15 +1,14 @@ +import { isVariableWithAliasReference } from '@/utils/isAliasReference'; + export default function setBooleanValuesOnVariable(variable: Variable, mode: string, value: string) { try { const existingVariableValue = variable.valuesByMode[mode]; - if (typeof existingVariableValue !== 'boolean') return; + if (existingVariableValue === undefined || !(typeof existingVariableValue === 'boolean' || isVariableWithAliasReference(existingVariableValue))) return; + + const newValue = value === 'true'; - const existingValueString = existingVariableValue.toString(); - if (existingValueString !== value) { - if (value === 'true') { - variable.setValueForMode(mode, value === 'true'); - } else { - variable.setValueForMode(mode, false); - } + if (existingVariableValue !== newValue) { + variable.setValueForMode(mode, newValue); } } catch (e) { console.error('Error setting booleanVariable', e); diff --git a/packages/tokens-studio-for-figma/src/plugin/setColorValuesOnVariable.test.ts b/packages/tokens-studio-for-figma/src/plugin/setColorValuesOnVariable.test.ts new file mode 100644 index 000000000..4376713de --- /dev/null +++ b/packages/tokens-studio-for-figma/src/plugin/setColorValuesOnVariable.test.ts @@ -0,0 +1,78 @@ +import setColorValuesOnVariable, { normalizeFigmaColor } from './setColorValuesOnVariable'; + +describe('normalizeFigmaColor', () => { + it('should round color values to 6 decimal places', () => { + const input = { + r: 0.12345678, + g: 0.23456789, + b: 0.34567890, + a: 0.45678901, + }; + + const result = normalizeFigmaColor(input); + + expect(result).toEqual({ + r: 0.123457, + g: 0.234568, + b: 0.345679, + a: 0.456789, + }); + }); +}); + +describe('setColorValuesOnVariable', () => { + let mockVariable: Variable; + const mockMode = 'light'; + + beforeEach(() => { + mockVariable = { + valuesByMode: {}, + setValueForMode: jest.fn(), + } as unknown as Variable; + }); + + it('should set new color value when values are different', () => { + mockVariable.valuesByMode[mockMode] = { + r: 0, g: 0, b: 0, a: 1, + }; + + setColorValuesOnVariable(mockVariable, mockMode, '#FF0000'); + + expect(mockVariable.setValueForMode).toHaveBeenCalledWith( + mockMode, + expect.objectContaining({ + r: 1, g: 0, b: 0, a: 1, + }), + ); + }); + + it('should not set value when colors are identical', () => { + mockVariable.valuesByMode[mockMode] = { + r: 1, g: 0, b: 0, a: 1, + }; + + setColorValuesOnVariable(mockVariable, mockMode, '#FF0000'); + + expect(mockVariable.setValueForMode).not.toHaveBeenCalled(); + }); + + it('should handle invalid existing variable values', () => { + mockVariable.valuesByMode[mockMode] = 'invalid' as any; + + setColorValuesOnVariable(mockVariable, mockMode, '#FF0000'); + + expect(mockVariable.setValueForMode).not.toHaveBeenCalled(); + }); + + it('should handle errors gracefully', () => { + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + setColorValuesOnVariable(mockVariable, mockMode, 'invalid-color'); + + expect(consoleSpy).toHaveBeenCalledWith( + 'Error setting colorVariable', + expect.any(Error), + ); + consoleSpy.mockRestore(); + }); +}); diff --git a/packages/tokens-studio-for-figma/src/plugin/setColorValuesOnVariable.ts b/packages/tokens-studio-for-figma/src/plugin/setColorValuesOnVariable.ts index 13aa23107..6a0050ee1 100644 --- a/packages/tokens-studio-for-figma/src/plugin/setColorValuesOnVariable.ts +++ b/packages/tokens-studio-for-figma/src/plugin/setColorValuesOnVariable.ts @@ -1,3 +1,4 @@ +import { isVariableWithAliasReference } from '@/utils/isAliasReference'; import { convertToFigmaColor } from './figmaTransforms/colors'; export function normalizeFigmaColor({ @@ -22,16 +23,22 @@ export default function setColorValuesOnVariable(variable: Variable, mode: strin try { const { color, opacity } = convertToFigmaColor(value); const existingVariableValue = variable.valuesByMode[mode]; - if (!existingVariableValue || !isFigmaColorObject(existingVariableValue)) return; - const existingValue = normalizeFigmaColor(existingVariableValue as RGBA); - const newValue = normalizeFigmaColor({ ...color, a: opacity }); + if (!existingVariableValue || !(isFigmaColorObject(existingVariableValue) || isVariableWithAliasReference(existingVariableValue))) return; - if ((existingValue.r !== newValue.r) - || (existingValue.g !== newValue.g) - || (existingValue.b !== newValue.b) - || (existingValue.a !== newValue.a)) { - variable.setValueForMode(mode, newValue); + const existingValue = isFigmaColorObject(existingVariableValue) ? normalizeFigmaColor(existingVariableValue) : existingVariableValue; + const newValue = isFigmaColorObject(color) ? normalizeFigmaColor({ ...color, a: opacity }) : { ...color, a: opacity }; + + if (isFigmaColorObject(existingValue) && isFigmaColorObject(newValue)) { + if ((existingValue.r === newValue.r) + && (existingValue.g === newValue.g) + && (existingValue.b === newValue.b) + && (existingValue.a === newValue.a)) { + // return if values match + return; + } } + + variable.setValueForMode(mode, newValue); } catch (e) { console.error('Error setting colorVariable', e); } diff --git a/packages/tokens-studio-for-figma/src/plugin/setNumberValuesOnVariable.test.ts b/packages/tokens-studio-for-figma/src/plugin/setNumberValuesOnVariable.test.ts new file mode 100644 index 000000000..6852c1780 --- /dev/null +++ b/packages/tokens-studio-for-figma/src/plugin/setNumberValuesOnVariable.test.ts @@ -0,0 +1,68 @@ +import setNumberValuesOnVariable from './setNumberValuesOnVariable'; +import { isVariableWithAliasReference } from '@/utils/isAliasReference'; + +jest.mock('@/utils/isAliasReference'); + +describe('setNumberValuesOnVariable', () => { + let mockVariable: Variable; + const mockMode = 'light'; + + beforeEach(() => { + mockVariable = { + name: 'testVariable', + valuesByMode: { + light: 10, + }, + setValueForMode: jest.fn(), + } as unknown as Variable; + + (isVariableWithAliasReference as unknown as jest.Mock).mockReset(); + }); + + it('should set new number value when different from existing value', () => { + (isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false); + setNumberValuesOnVariable(mockVariable, mockMode, 20); + expect(mockVariable.setValueForMode).toHaveBeenCalledWith(mockMode, 20); + }); + + it('should not set value when new value equals existing value', () => { + (isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false); + setNumberValuesOnVariable(mockVariable, mockMode, 10); + expect(mockVariable.setValueForMode).not.toHaveBeenCalled(); + }); + + it('should handle alias references', () => { + mockVariable.valuesByMode.light = '{number.primary}'; + (isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(true); + setNumberValuesOnVariable(mockVariable, mockMode, 30); + expect(mockVariable.setValueForMode).toHaveBeenCalledWith(mockMode, 30); + }); + + it('should handle NaN values', () => { + const consoleSpy = jest.spyOn(console, 'error'); + setNumberValuesOnVariable(mockVariable, mockMode, NaN); + expect(mockVariable.setValueForMode).not.toHaveBeenCalled(); + expect(consoleSpy).toHaveBeenCalled(); + }); + + it('should handle existing value of 0', () => { + mockVariable.valuesByMode.light = 0; + (isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false); + setNumberValuesOnVariable(mockVariable, mockMode, 20); + expect(mockVariable.setValueForMode).toHaveBeenCalledWith(mockMode, 20); + }); + + it('should handle new value of 0', () => { + mockVariable.valuesByMode.light = 20; + (isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false); + setNumberValuesOnVariable(mockVariable, mockMode, 0); + expect(mockVariable.setValueForMode).toHaveBeenCalledWith(mockMode, 0); + }); + + it('should return early if existing value is invalid', () => { + mockVariable.valuesByMode.light = 'invalid' as any; + (isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false); + setNumberValuesOnVariable(mockVariable, mockMode, 20); + expect(mockVariable.setValueForMode).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/tokens-studio-for-figma/src/plugin/setNumberValuesOnVariable.ts b/packages/tokens-studio-for-figma/src/plugin/setNumberValuesOnVariable.ts index 03544c927..9bf1497b2 100644 --- a/packages/tokens-studio-for-figma/src/plugin/setNumberValuesOnVariable.ts +++ b/packages/tokens-studio-for-figma/src/plugin/setNumberValuesOnVariable.ts @@ -1,12 +1,17 @@ +import { isVariableWithAliasReference } from '@/utils/isAliasReference'; + export default function setNumberValuesOnVariable(variable: Variable, mode: string, value: number) { try { if (isNaN(value)) { throw new Error(`Skipping due to invalid value: ${value}`); } - const existingValue = variable.valuesByMode[mode]; - if (typeof existingValue !== 'number') return; - if (existingValue !== value) { - variable.setValueForMode(mode, value); + const existingVariableValue = variable.valuesByMode[mode]; + if (existingVariableValue === undefined || !(typeof existingVariableValue === 'number' || isVariableWithAliasReference(existingVariableValue))) return; + + const newValue = value; + + if (existingVariableValue !== newValue) { + variable.setValueForMode(mode, newValue); } } catch (e) { console.error('Error setting numberVariable on variable', variable.name, e); diff --git a/packages/tokens-studio-for-figma/src/plugin/setStringValuesOnVariable.test.ts b/packages/tokens-studio-for-figma/src/plugin/setStringValuesOnVariable.test.ts new file mode 100644 index 000000000..a02bbd3ec --- /dev/null +++ b/packages/tokens-studio-for-figma/src/plugin/setStringValuesOnVariable.test.ts @@ -0,0 +1,46 @@ +import setStringValuesOnVariable from './setStringValuesOnVariable'; +import { isVariableWithAliasReference } from '@/utils/isAliasReference'; + +jest.mock('@/utils/isAliasReference'); + +describe('setStringValuesOnVariable', () => { + let mockVariable: Variable; + const mockMode = 'light'; + + beforeEach(() => { + mockVariable = { + valuesByMode: { + light: 'oldValue', + }, + setValueForMode: jest.fn(), + } as unknown as Variable; + + (isVariableWithAliasReference as unknown as jest.Mock).mockReset(); + }); + + it('should set new string value when different from existing value', () => { + (isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false); + setStringValuesOnVariable(mockVariable, mockMode, 'newValue'); + expect(mockVariable.setValueForMode).toHaveBeenCalledWith(mockMode, 'newValue'); + }); + + it('should not set value when new value equals existing value', () => { + (isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false); + setStringValuesOnVariable(mockVariable, mockMode, 'oldValue'); + expect(mockVariable.setValueForMode).not.toHaveBeenCalled(); + }); + + it('should handle alias references', () => { + mockVariable.valuesByMode.light = '{color.primary}'; + (isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(true); + setStringValuesOnVariable(mockVariable, mockMode, '{color.secondary}'); + expect(mockVariable.setValueForMode).toHaveBeenCalledWith(mockMode, '{color.secondary}'); + }); + + it('should return early if existing value is invalid', () => { + mockVariable.valuesByMode.light = 123 as any; + (isVariableWithAliasReference as unknown as jest.Mock).mockReturnValue(false); + setStringValuesOnVariable(mockVariable, mockMode, 'newValue'); + expect(mockVariable.setValueForMode).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/tokens-studio-for-figma/src/plugin/setStringValuesOnVariable.ts b/packages/tokens-studio-for-figma/src/plugin/setStringValuesOnVariable.ts index 3204a3d30..313838ab3 100644 --- a/packages/tokens-studio-for-figma/src/plugin/setStringValuesOnVariable.ts +++ b/packages/tokens-studio-for-figma/src/plugin/setStringValuesOnVariable.ts @@ -1,9 +1,11 @@ +import { isVariableWithAliasReference } from '@/utils/isAliasReference'; + export default function setStringValuesOnVariable(variable: Variable, mode: string, value: string) { try { - const existingValue = variable.valuesByMode[mode]; - if (typeof existingValue !== 'string') return; + const existingVariableValue = variable.valuesByMode[mode]; + if (!existingVariableValue || !(typeof existingVariableValue === 'string' || isVariableWithAliasReference(existingVariableValue))) return; - if (existingValue !== value) { + if (existingVariableValue !== value) { variable.setValueForMode(mode, value); } } catch (e) { diff --git a/packages/tokens-studio-for-figma/src/utils/isAliasReference.ts b/packages/tokens-studio-for-figma/src/utils/isAliasReference.ts new file mode 100644 index 000000000..ff89a9631 --- /dev/null +++ b/packages/tokens-studio-for-figma/src/utils/isAliasReference.ts @@ -0,0 +1,3 @@ +export function isVariableWithAliasReference(obj: VariableValue): obj is VariableAlias { + return typeof obj === 'object' && 'id' in obj && typeof obj.id === 'string'; +} From 1f7988b0e1191da310f5356beec16cd51b91a869 Mon Sep 17 00:00:00 2001 From: Jan Six Date: Wed, 4 Dec 2024 09:25:47 +0100 Subject: [PATCH 2/2] add changeset --- .changeset/seven-keys-tease.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/seven-keys-tease.md diff --git a/.changeset/seven-keys-tease.md b/.changeset/seven-keys-tease.md new file mode 100644 index 000000000..1072ae80c --- /dev/null +++ b/.changeset/seven-keys-tease.md @@ -0,0 +1,5 @@ +--- +"@tokens-studio/figma-plugin": patch +--- + +Fixes an issue when updating a token's value from a reference to a hard value, the check we introduced in the last release caused those to not be updated (and only worked when changing to another reference).