From e16503a2abc8c77e37dc62980773930ec0d2a711 Mon Sep 17 00:00:00 2001 From: James Yang Date: Thu, 24 Aug 2023 14:26:27 -0700 Subject: [PATCH] Remove color rounding during token import (#11) Remove color rounding during token import now that we've rolled out the REST API fix for high-precision color values Also handle empty files and add a bunch more tests. Fixes: https://app.asana.com/0/1203505314277408/1205152183539898/f ## Testing You can run `npm run sync-tokens-to-figma` locally after making temporary changes to hex values in the tokens files, and check the variables authoring modal in the Figma UI to see that you get the same hex value. --- src/token_import.test.ts | 285 +++++++++++++++++++++++++++++++++++++-- src/token_import.ts | 12 +- 2 files changed, 282 insertions(+), 15 deletions(-) diff --git a/src/token_import.test.ts b/src/token_import.test.ts index d23453d..8fc1e21 100644 --- a/src/token_import.test.ts +++ b/src/token_import.test.ts @@ -44,14 +44,20 @@ jest.mock('fs', () => { 'no_tokens.mode1.json': JSON.stringify({ foo: 'bar', }), + 'empty_file.mode1.json': '', + 'file_with_$_keys.mode1.json': JSON.stringify({ + $foo: 'bar', + token1: { $type: 'string', $value: 'value1' }, + }), } return { readFileSync: (fpath: string) => { if (fpath in MOCK_FILE_INFO) { return MOCK_FILE_INFO[fpath] + } else { + return '{}' } - throw 'unexpected fpath' }, } }) @@ -83,6 +89,37 @@ describe('readJsonFiles', () => { const result = readJsonFiles(['no_tokens.mode1.json']) expect(result).toEqual({ 'no_tokens.mode1.json': {} }) }) + + it('handles duplicate collections and modes', () => { + expect(() => { + readJsonFiles([ + 'tokens/collection1.mode1.1.json', + 'tokens/collection1.mode1.2.json', + 'tokens/collection1.mode1.3.json', + ]) + }).toThrowError('Duplicate collection and mode in file: tokens/collection1.mode1.2.json') + }) + + it('handles file names that do not match the expected format', () => { + expect(() => { + readJsonFiles(['tokens/collection1.mode1.json', 'tokens/collection2.mode1.json', 'foo.json']) + }).toThrowError( + 'Invalid tokens file name: foo.json. File names must be in the format: {collectionName}.{modeName}.json', + ) + }) + + it('ignores keys that start with $', () => { + const result = readJsonFiles(['file_with_$_keys.mode1.json']) + expect(result).toEqual({ + 'file_with_$_keys.mode1.json': { token1: { $type: 'string', $value: 'value1' } }, + }) + }) + + it('handles empty files', () => { + expect(() => { + readJsonFiles(['empty_file.mode1.json']) + }).toThrowError('Invalid tokens file: empty_file.mode1.json. File is empty.') + }) }) describe('generatePostVariablesPayload', () => { @@ -219,12 +256,12 @@ describe('generatePostVariablesPayload', () => { { variableId: 'color/brand/radish', modeId: 'mode1', - value: { r: 1, g: 0.7451, b: 0.08627 }, + value: { r: 1, g: 0.7450980392156863, b: 0.08627450980392157 }, }, { variableId: 'color/brand/pear', modeId: 'mode1', - value: { r: 1, g: 0.7451, b: 0.08627 }, + value: { r: 1, g: 0.7450980392156863, b: 0.08627450980392157 }, }, // primitives, mode2 @@ -233,12 +270,12 @@ describe('generatePostVariablesPayload', () => { { variableId: 'color/brand/radish', modeId: 'mode2', - value: { r: 0.00392, g: 0.00392, b: 0.00392 }, + value: { r: 0.00392156862745098, g: 0.00392156862745098, b: 0.00392156862745098 }, }, { variableId: 'color/brand/pear', modeId: 'mode2', - value: { r: 0.00392, g: 0.00392, b: 0.00392 }, + value: { r: 0.00392156862745098, g: 0.00392156862745098, b: 0.00392156862745098 }, }, // tokens, mode1 @@ -456,7 +493,7 @@ describe('generatePostVariablesPayload', () => { { variableId: 'VariableID:2:4', modeId: '1:0', - value: { r: 1, g: 0.7451, b: 0.08627 }, + value: { r: 1, g: 0.7450980392156863, b: 0.08627450980392157 }, }, // primitives, mode2 @@ -465,12 +502,12 @@ describe('generatePostVariablesPayload', () => { { variableId: 'VariableID:2:3', modeId: 'mode2', - value: { r: 0.00392, g: 0.00392, b: 0.00392 }, + value: { r: 0.00392156862745098, g: 0.00392156862745098, b: 0.00392156862745098 }, }, { variableId: 'VariableID:2:4', modeId: 'mode2', - value: { r: 0.00392, g: 0.00392, b: 0.00392 }, + value: { r: 0.00392156862745098, g: 0.00392156862745098, b: 0.00392156862745098 }, }, // tokens, mode1 @@ -499,6 +536,189 @@ describe('generatePostVariablesPayload', () => { ]) }) + it('noops when everything is already in sync (with aliases)', () => { + const localVariablesResponse: ApiGetLocalVariablesResponse = { + status: 200, + error: false, + meta: { + variableCollections: { + 'VariableCollectionId:1:1': { + id: 'VariableCollectionId:1:1', + name: 'collection', + modes: [{ modeId: '1:0', name: 'mode1' }], + defaultModeId: '1:0', + remote: false, + hiddenFromPublishing: false, + }, + }, + variables: { + 'VariableID:2:1': { + id: 'VariableID:2:1', + name: 'var1', + key: 'variable_key', + variableCollectionId: 'VariableCollectionId:1:1', + resolvedType: 'STRING', + valuesByMode: { + '1:0': 'hello world!', + }, + remote: false, + description: '', + hiddenFromPublishing: false, + scopes: ['ALL_SCOPES'], + codeSyntax: {}, + }, + 'VariableID:2:2': { + id: 'VariableID:2:2', + name: 'var2', + key: 'variable_key2', + variableCollectionId: 'VariableCollectionId:1:1', + resolvedType: 'STRING', + valuesByMode: { + '1:0': { type: 'VARIABLE_ALIAS', id: 'VariableID:2:1' }, + }, + remote: false, + description: '', + hiddenFromPublishing: false, + scopes: ['ALL_SCOPES'], + codeSyntax: {}, + }, + }, + }, + } + + const tokensByFile: FlattenedTokensByFile = { + 'collection.mode1.json': { + var1: { + $type: 'string', + $value: 'hello world!', + $description: '', + $extensions: { + 'com.figma': { + hiddenFromPublishing: false, + scopes: ['ALL_SCOPES'], + codeSyntax: {}, + }, + }, + }, + var2: { + $type: 'string', + $value: '{var1}', + $description: '', + $extensions: { + 'com.figma': { + hiddenFromPublishing: false, + scopes: ['ALL_SCOPES'], + codeSyntax: {}, + }, + }, + }, + }, + } + + const result = generatePostVariablesPayload(tokensByFile, localVariablesResponse) + + expect(result).toEqual({ + variableCollections: [], + variableModes: [], + variables: [], + variableModeValues: [], + }) + }) + + it('ignores remote collections and variables', () => { + const localVariablesResponse: ApiGetLocalVariablesResponse = { + status: 200, + error: false, + meta: { + variableCollections: { + 'VariableCollectionId:1:1': { + id: 'VariableCollectionId:1:1', + name: 'collection', + modes: [{ modeId: '1:0', name: 'mode1' }], + defaultModeId: '1:0', + remote: true, + hiddenFromPublishing: false, + }, + }, + variables: { + 'VariableID:2:1': { + id: 'VariableID:2:1', + name: 'var1', + key: 'variable_key', + variableCollectionId: 'VariableCollectionId:1:1', + resolvedType: 'STRING', + valuesByMode: { + '1:0': 'hello world!', + }, + remote: true, + description: '', + hiddenFromPublishing: false, + scopes: ['ALL_SCOPES'], + codeSyntax: {}, + }, + 'VariableID:2:2': { + id: 'VariableID:2:2', + name: 'var2', + key: 'variable_key2', + variableCollectionId: 'VariableCollectionId:1:1', + resolvedType: 'STRING', + valuesByMode: { + '1:0': { type: 'VARIABLE_ALIAS', id: 'VariableID:2:1' }, + }, + remote: true, + description: '', + hiddenFromPublishing: false, + scopes: ['ALL_SCOPES'], + codeSyntax: {}, + }, + }, + }, + } + + const tokensByFile: FlattenedTokensByFile = { + 'collection.mode1.json': { + var1: { + $type: 'string', + $value: 'hello world!', + $description: '', + $extensions: { + 'com.figma': { + hiddenFromPublishing: false, + scopes: ['ALL_SCOPES'], + codeSyntax: {}, + }, + }, + }, + var2: { + $type: 'string', + $value: '{var1}', + $description: '', + $extensions: { + 'com.figma': { + hiddenFromPublishing: false, + scopes: ['ALL_SCOPES'], + codeSyntax: {}, + }, + }, + }, + }, + } + + const result = generatePostVariablesPayload(tokensByFile, localVariablesResponse) + + // Since all existing collections and variables are remote, result should be equivalent to an initial sync + expect(result).toEqual( + generatePostVariablesPayload(tokensByFile, { + status: 200, + error: false, + meta: { + variableCollections: {}, + variables: {}, + }, + }), + ) + }) + it('throws on unsupported token types', async () => { const localVariablesResponse = { status: 200, @@ -519,4 +739,53 @@ describe('generatePostVariablesPayload', () => { generatePostVariablesPayload(tokensByFile, localVariablesResponse) }).toThrowError('Invalid token $type: fontWeight') }) + + it('throws on duplicate variable collections in the Figma file', () => { + const localVariablesResponse: ApiGetLocalVariablesResponse = { + status: 200, + error: false, + meta: { + variableCollections: { + 'VariableCollectionId:1:1': { + id: 'VariableCollectionId:1:1', + name: 'collection', + modes: [{ modeId: '1:0', name: 'mode1' }], + defaultModeId: '1:0', + remote: false, + hiddenFromPublishing: false, + }, + 'VariableCollectionId:1:2': { + id: 'VariableCollectionId:1:2', + name: 'collection', + modes: [{ modeId: '2:0', name: 'mode1' }], + defaultModeId: '2:0', + remote: false, + hiddenFromPublishing: false, + }, + }, + variables: {}, + }, + } + + const tokensByFile: FlattenedTokensByFile = { + 'collection.mode1.json': { + var1: { + $type: 'string', + $value: 'hello world!', + $description: '', + $extensions: { + 'com.figma': { + hiddenFromPublishing: false, + scopes: ['ALL_SCOPES'], + codeSyntax: {}, + }, + }, + }, + }, + } + + expect(() => { + generatePostVariablesPayload(tokensByFile, localVariablesResponse) + }).toThrowError('Duplicate variable collection in file: collection') + }) }) diff --git a/src/token_import.ts b/src/token_import.ts index 8fdbea5..2caf9e2 100644 --- a/src/token_import.ts +++ b/src/token_import.ts @@ -36,6 +36,10 @@ export function readJsonFiles(files: string[]) { seenCollectionsAndModes.add(`${collectionName}.${modeName}`) const fileContent = fs.readFileSync(file, { encoding: 'utf-8' }) + + if (!fileContent) { + throw new Error(`Invalid tokens file: ${file}. File is empty.`) + } const tokensFile: TokensFile = JSON.parse(fileContent) tokensJsonByFile[baseFileName] = flattenTokensFile(tokensFile) @@ -146,13 +150,7 @@ function variableValueFromToken( id: value, } } else if (typeof token.$value === 'string' && token.$type === 'color') { - const color = parseColor(token.$value) - // TODO: remove the rounding once we fix the POST variables bug where it can't handle - // color values with more than 16 decimal places - color.r = Math.round(color.r * 100000) / 100000 - color.g = Math.round(color.g * 100000) / 100000 - color.b = Math.round(color.b * 100000) / 100000 - return color + return parseColor(token.$value) } else { return token.$value }