Skip to content

fix font export#3928

Open
akshay-gupta7 wants to merge 5 commits into
mainfrom
akshay/fix-font-export
Open

fix font export#3928
akshay-gupta7 wants to merge 5 commits into
mainfrom
akshay/fix-font-export

Conversation

@akshay-gupta7

@akshay-gupta7 akshay-gupta7 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Why does this PR exist?

Font-family tokens resolved by the Tokens Studio server (Rails/gRPC path) come through as an array-shaped string — e.g. '["Arial","Helvetica","sans-serif"]'. On Export to Variables, that string was being written to the Figma variable as the literal [Arial, because convertFontFamilyToFigma split by , and stripped quotes without ever removing the surrounding brackets. Font-weights had the same latent issue via convertFontWeightToFigma.

What does this pull request do?

  • convertFontFamilyToFigma — strips surrounding [...] before splitting. For JSON-shaped arrays (["A","B"]) tries JSON.parse first so quoted family names containing commas (["Font, Name","Arial"]Font, Name) survive; falls back to a split for single-quoted / unquoted bracket lists.
  • convertFontWeightToFigma — mirrors the bracket-strip so server-form '["Bold","Regular"]' no longer flows through untouched.
  • setValuesOnVariable — STRING case simplified. Normalization now lives in the transform functions (right altitude), so the switch only needs a plain-string branch plus the array branch. Numeric-array coercion via String() is kept as a belt-and-suspenders safety net.

Testing this change

  1. Connect a Studio-server project that has a font-family token with multiple fallbacks (or a hardcoded ["Arial","Helvetica","sans-serif"]-shaped value).
  2. Run Export to Variables.
  3. Verify the Figma variable value is Arial (previously [Arial).
  4. Repeat with a font-weight token whose value is an array-shaped string.
  5. Unit tests — yarn jest --testPathPattern="convertFontFamilyToFigma|fontWeight\.test|setValuesOnVariable" — 40 tests across 4 suites pass, including new coverage for JSON, single-quoted, and unquoted bracket forms plus the comma-in-quoted-name case.

Additional Notes (if any)

Addresses Copilot review comments (silent JSON parse, added tests for both transforms) and follow-up ultra-review findings (fontWeights parity gap, dead JSON-string branch in setValuesOnVariable, "[]" literal write, comma-in-quoted-name).

@changeset-bot

changeset-bot Bot commented Jul 21, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 440b886

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@tokens-studio/figma-plugin Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

⤵️ 📦 ✨ The artifact was successfully created! Want to test it? Download it here 👀 🎁

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to fix font-related exports by improving how font-family/font-weight token values are normalized when they arrive in “array-shaped string” formats (e.g. ["Arial","Helvetica"]) so that Figma variables receive a single usable string value.

Changes:

  • Update setValuesOnVariable to detect font family/weight “array-shaped string” values and extract the first entry before setting string variables.
  • Update convertFontFamilyToFigma to strip surrounding brackets from array-shaped font-family strings before splitting and selecting the first family.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
packages/tokens-studio-for-figma/src/plugin/setValuesOnVariable.ts Extract first entry from array-shaped font family/weight values before writing to STRING variables.
packages/tokens-studio-for-figma/src/plugin/figmaTransforms/convertFontFamilyToFigma.ts Strip [/] from bracketed font-family strings before splitting to avoid returning "[Arial"-style results.

Comment thread packages/tokens-studio-for-figma/src/plugin/setValuesOnVariable.ts Outdated
Comment thread packages/tokens-studio-for-figma/src/plugin/setValuesOnVariable.ts Outdated
Comment thread packages/tokens-studio-for-figma/src/plugin/setValuesOnVariable.ts Outdated
@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Commit SHA:885e339cb9dde1424b49721ce135a2aa0dffb309

Test coverage results 🧪

Code coverage diff between base branch:main and head branch: akshay/fix-font-export 
Status File % Stmts % Branch % Funcs % Lines
🟢 total 59.88 (0.05) 51.87 (0.11) 57.84 (0) 60.38 (0.05)
🟢 packages/tokens-studio-for-figma/src/plugin/setValuesOnVariable.ts 85.86 (1.33) 79.14 (1.96) 82.35 (0) 86.11 (1.37)
🟢 packages/tokens-studio-for-figma/src/plugin/figmaTransforms/convertFontFamilyToFigma.ts 88.88 (13.88) 76.47 (9.81) 100 (0) 88.88 (13.88)
🔴 packages/tokens-studio-for-figma/src/plugin/figmaTransforms/fontWeight.ts 100 (0) 95.23 (-4.77) 100 (0) 100 (0)
🟢 packages/tokens-studio-for-figma/src/utils/convertTokenTypeToVariableType.ts 88.88 (0) 86.66 (6.66) 100 (0) 88.88 (0)

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Commit SHA:885e339cb9dde1424b49721ce135a2aa0dffb309
Current PR reduces the test coverage percentage by 1 for some tests

@akshay-gupta7

Copy link
Copy Markdown
Contributor Author

Code Review: PR-3928

Effort: max · Scope: main...pr-3928 (4 files, 171 diff lines)


1. FONT_WEIGHTS parity gap — same bug still present ⚠️ CONFIRMED

File: packages/tokens-studio-for-figma/src/plugin/setValuesOnVariable.ts:254

The array-shaped-string bug this PR fixes for fontFamilies is not fixed for fontWeights.

Trace for input token.value = '["Bold","Regular"]' on a FONT_WEIGHTS STRING variable:

  1. mapTokensToVariableInfo calls transformValue(value, 'fontWeights', ..., true)
  2. transformValueconvertFontWeightToFigma(value, true) → returns [value] (fontWeight.ts:2-3 just wraps the input)
  3. token.value is now ['["Bold","Regular"]']
  4. setValuesOnVariable hits the Array.isArray && typeof value[0] === 'string' branch
  5. Writes '["Bold","Regular"]' verbatim to the variable

Fix: mirror the bracket-strip in convertFontWeightToFigma, or handle it in the pre-transform step.


2. Dead JSON-string branch + misleading test ⚠️ CONFIRMED

File: packages/tokens-studio-for-figma/src/plugin/setValuesOnVariable.ts:234

The tryParseJson/splitter branch inside setValuesOnVariable is unreachable in production for FONT_FAMILIES:

  • mapTokensToVariableInfo (utils/mapTokensToVariableInfo.ts:18) runs transformValue on string values
  • transformValue routes fontFamilies through convertFontFamilyToFigma(..., true), which already strips brackets and returns a single family
  • By the time the STRING case runs, a fontFamilies token.value can never still be '["Arial","Helvetica"]'

The unit test 'FONT_FAMILIES JSON-array string' passes only because it calls setValuesOnVariable directly, bypassing the real pipeline — it gives false confidence the fix works end-to-end.

Fix: either delete the FONT_FAMILIES portion of the setValuesOnVariable branch, or (better) route the test through mapTokensToVariableInfo so it validates the actual production path.


3. Empty-array bracket string writes literal "[]" ⚠️ CONFIRMED

File: packages/tokens-studio-for-figma/src/plugin/setValuesOnVariable.ts:244

For token.value = '[]':

  • trimmed starts/ends with brackets → enter parse branch
  • tryParseJson skipped (no ")
  • inner = '', firstRaw = '', firstUnquoted = '' → length 0
  • Fallback: stringValue = token.value = '[]'
  • Variable ends up with literal "[]" as its font family / weight

Fix: when the fallback yields empty, set stringValue = undefined so the write is skipped entirely.


4. Numeric FONT_WEIGHTS arrays silently dropped 🟡 PLAUSIBLE

File: packages/tokens-studio-for-figma/src/plugin/setValuesOnVariable.ts:246

Old code:

else if (token.type === TokenTypes.FONT_WEIGHTS && Array.isArray(token.value)) {
  setStringValuesOnVariable(variable, mode, token.value[0], hasMetadataChanged);
}

New code additionally requires typeof token.value[0] === 'string'.

Input [400, 500] on a STRING variable now falls through with stringValue === undefined and the write is skipped. Previously it would have called setStringValuesOnVariable(..., 400).

Very edge case (numeric weights typically create FLOAT variables), but no test locks the behavior in either direction — worth an explicit decision.


5. Quoted family names containing a comma get truncated 🟡 PLAUSIBLE

File: packages/tokens-studio-for-figma/src/plugin/figmaTransforms/convertFontFamilyToFigma.ts:12

For input '["Font, Name","Arial"]':

  • withoutBrackets = '"Font, Name","Arial"'
  • split(',')['"Font', ' Name"', '"Arial"']
  • First element .trim().replace(/['"]/g,'')'Font'

Variable ends up with Font instead of Font, Name. Pre-existing bug for non-bracketed input, but the PR's example ('["Arial","Helvetica"]') proves the server can emit JSON — where quoted-with-comma names are legal.

Fix: for bracketed inputs, attempt JSON.parse first (mirroring the approach in setValuesOnVariable); fall back to split only on parse failure.


Priority

# Severity Action
1 High — real user-visible bug on the sibling code path Fix before merge
2 Medium — dead code + false-signal test Fix before merge, or file follow-up
3 Low — narrow edge case Fix in this PR (one-liner)
4 Low — behavior change, undocumented Decide + add test
5 Low — pre-existing, but PR's context makes it newly relevant Follow-up PR OK

@akshay-gupta7

Copy link
Copy Markdown
Contributor Author

Code Review: PR-3928

Effort: max · Scope: main...pr-3928 (4 files, 171 diff lines)


1. FONT_WEIGHTS parity gap — same bug still present ⚠️ CONFIRMED

File: packages/tokens-studio-for-figma/src/plugin/setValuesOnVariable.ts:254

The array-shaped-string bug this PR fixes for fontFamilies is not fixed for fontWeights.

Trace for input token.value = '["Bold","Regular"]' on a FONT_WEIGHTS STRING variable:

  1. mapTokensToVariableInfo calls transformValue(value, 'fontWeights', ..., true)
  2. transformValueconvertFontWeightToFigma(value, true) → returns [value] (fontWeight.ts:2-3 just wraps the input)
  3. token.value is now ['["Bold","Regular"]']
  4. setValuesOnVariable hits the Array.isArray && typeof value[0] === 'string' branch
  5. Writes '["Bold","Regular"]' verbatim to the variable

Fix: mirror the bracket-strip in convertFontWeightToFigma, or handle it in the pre-transform step.


2. Dead JSON-string branch + misleading test ⚠️ CONFIRMED

File: packages/tokens-studio-for-figma/src/plugin/setValuesOnVariable.ts:234

The tryParseJson/splitter branch inside setValuesOnVariable is unreachable in production for FONT_FAMILIES:

  • mapTokensToVariableInfo (utils/mapTokensToVariableInfo.ts:18) runs transformValue on string values
  • transformValue routes fontFamilies through convertFontFamilyToFigma(..., true), which already strips brackets and returns a single family
  • By the time the STRING case runs, a fontFamilies token.value can never still be '["Arial","Helvetica"]'

The unit test 'FONT_FAMILIES JSON-array string' passes only because it calls setValuesOnVariable directly, bypassing the real pipeline — it gives false confidence the fix works end-to-end.

Fix: either delete the FONT_FAMILIES portion of the setValuesOnVariable branch, or (better) route the test through mapTokensToVariableInfo so it validates the actual production path.


3. Empty-array bracket string writes literal "[]" ⚠️ CONFIRMED

File: packages/tokens-studio-for-figma/src/plugin/setValuesOnVariable.ts:244

For token.value = '[]':

  • trimmed starts/ends with brackets → enter parse branch
  • tryParseJson skipped (no ")
  • inner = '', firstRaw = '', firstUnquoted = '' → length 0
  • Fallback: stringValue = token.value = '[]'
  • Variable ends up with literal "[]" as its font family / weight

Fix: when the fallback yields empty, set stringValue = undefined so the write is skipped entirely.


4. Numeric FONT_WEIGHTS arrays silently dropped 🟡 PLAUSIBLE

File: packages/tokens-studio-for-figma/src/plugin/setValuesOnVariable.ts:246

Old code:

else if (token.type === TokenTypes.FONT_WEIGHTS && Array.isArray(token.value)) {
  setStringValuesOnVariable(variable, mode, token.value[0], hasMetadataChanged);
}

New code additionally requires typeof token.value[0] === 'string'.

Input [400, 500] on a STRING variable now falls through with stringValue === undefined and the write is skipped. Previously it would have called setStringValuesOnVariable(..., 400).

Very edge case (numeric weights typically create FLOAT variables), but no test locks the behavior in either direction — worth an explicit decision.


5. Quoted family names containing a comma get truncated 🟡 PLAUSIBLE

File: packages/tokens-studio-for-figma/src/plugin/figmaTransforms/convertFontFamilyToFigma.ts:12

For input '["Font, Name","Arial"]':

  • withoutBrackets = '"Font, Name","Arial"'
  • split(',')['"Font', ' Name"', '"Arial"']
  • First element .trim().replace(/['"]/g,'')'Font'

Variable ends up with Font instead of Font, Name. Pre-existing bug for non-bracketed input, but the PR's example ('["Arial","Helvetica"]') proves the server can emit JSON — where quoted-with-comma names are legal.

Fix: for bracketed inputs, attempt JSON.parse first (mirroring the approach in setValuesOnVariable); fall back to split only on parse failure.


Priority

# Severity Action
1 High — real user-visible bug on the sibling code path Fix before merge
2 Medium — dead code + false-signal test Fix before merge, or file follow-up
3 Low — narrow edge case Fix in this PR (one-liner)
4 Low — behavior change, undocumented Decide + add test
5 Low — pre-existing, but PR's context makes it newly relevant Follow-up PR OK

@anathaniel-TS

Copy link
Copy Markdown

🛡️ Hyma Compliance Check

No compliance-relevant changes detected

7 file(s) scanned · 0 findings

ℹ️ This check is informational.
🛡️ Hyma Compliance Check · automated

@anathaniel-TS

Copy link
Copy Markdown

🛡️ Hyma Compliance Check

No compliance-relevant changes detected

3 file(s) scanned · 0 findings

ℹ️ This check is informational.
🛡️ Hyma Compliance Check · automated

@anathaniel-TS

Copy link
Copy Markdown

🛡️ Hyma Compliance Check

No compliance-relevant changes detected

7 file(s) scanned · 0 findings

ℹ️ This check is informational.
🛡️ Hyma Compliance Check · automated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants