fix: resolve variable modes independently on export for Tokens Studio…#3927
fix: resolve variable modes independently on export for Tokens Studio…#3927akshay-gupta7 wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: da703de The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
|
|
Commit SHA:43e301cc10fb028d303af35aa687dc16448351fd Test coverage results 🧪
|
|
Commit SHA:43e301cc10fb028d303af35aa687dc16448351fd |
Code Review — Per-Theme Server-Resolved Tokens FixBranch: CONFIRMED (fix before merge)1. WITHOUT_MODES export path has the same bugFile:
Failure scenario: OAuth user with active theme 2. Per-theme fetch depends on transient UI stateFile: The helper layers the currently active theme's selections for OTHER groups. Exported variable values therefore depend on which theme the user happens to have active in the plugin, not on the export selection. Failure scenario: Groups 3. Regression test doesn't guard the actual bug siteFile: The new test hands pre-sliced per-theme deltas directly to Failure scenario: A future refactor reverts PLAUSIBLE (correctness — worth addressing)4. Silent partial-failure mixingFile:
Failure scenario: Export Light + Dark + HighContrast. HighContrast fetch hits a transient 502 → null. Result is 5. Lost fallback to Redux cache on transient failureFile: The old path used the debounced Redux Failure scenario: Redux delta was populated 30 s ago and is still fresh. Network briefly flaps. Old behavior: exports with the cached delta → correct. New behavior: fresh fetch returns null → local-only values for any token whose server-side resolution the client can't reproduce. 6. No try/finally around fetch → stuck spinner on errorFile: The fetch + dispatch chain isn't wrapped. Any throw leaves the infinite Failure scenario: 7. Fallback param name uses internal groupIdFile:
Failure scenario: Ungrouped project. Server sees 8. No requestId / abort guard around the fetchFile: If the user switches projects or triggers another export mid-fetch, the in-flight closure still completes and dispatches against newer Redux state. Failure scenario: User clicks Export in project A. While fetches are pending, they switch to project B. The A-closure completes, sends 9. Missing plugin-loop test for the actual fixFile: Companion to finding #3. The per-theme slicing Failure scenario: A future change swaps the key from 10. Silently drops stale activeTheme entriesFile:
11. Dropped dep is a footgun for future editsFile: The unused PLAUSIBLE (efficiency / UX / coverage)12. N parallel HTTP requests, no cache reuse, no dedup, no budgetFile: Every export click fires one request per selected theme with no interaction with the debounced Redux fetch already running. Perf and rate-limit regression vs the old zero-extra-request path. Scenario: 20-theme color-scheme group + "Export all themes" → 20 parallel GETs; any per-user rate limit may 429 some (each 429 becomes a silent null in the map). 13. Fetch runs under 'Creating variables' spinner with no cancelFile:
Scenario: 10 themes, ~8s tail → user may assume the plugin is stuck and reload mid-export. 14. Duplicated per-theme lookup at three call sitesFile:
15. Unit tests don't cover end-to-end failure recoveryFile: Coverage exercises the utility in isolation but never asserts that a Studio-side failure results in the export path falling back gracefully. WITHOUT_MODES equivalent is also uncovered. Recommended next steps
|
Per-Theme Variable Export Fix — Review Follow-up StatusBranch: Fixed (10 findings)CONFIRMED — all three addressed
PLAUSIBLE — seven addressed
Deferred (5 findings — documented, low ROI now)
Verification
Files touched (round 2)
|
|
🛡️ Hyma Compliance Check ✅ No compliance-relevant changes detected 8 file(s) scanned · 0 findings Note: adds per-theme fetches to the existing Tokens Studio API using the existing OAuth access token — no new external domain/subprocessor and design-token data only.
|
|
🛡️ Hyma Compliance Check ✅ No compliance-relevant changes detected 4 file(s) scanned · 0 findings
|
|
🛡️ Hyma Compliance Check
Full finding details[Access Control] OAuth access token forwarded in new per-theme token-resolution requests
|
… OAuth
Why does this PR exist?
Closes #0000
When exporting tokens to Figma Variables from a theme group that has multiple options (modes), every mode ended up with the same values instead of resolving distinctly per mode. Only Tokens Studio OAuth projects were affected — the server-resolved delta used to override local resolution was fetched once for the currently active theme and then applied to every mode in the export loop, clobbering per-mode values.
What does this pull request do?
Core fix — per-theme resolution in the variable export path
serverResolvedTokensonCREATE_LOCAL_VARIABLESis now a per-theme map (Record<themeId, Record<tokenName, value>>) instead of a single flat delta.fetchServerResolvedTokensPerThemeutility issues one server request per selected theme in parallel and returns the map keyed bytheme.id.createVariablesFromThemesinuseTokensbuilds the per-theme map before dispatch (OAuth projects only). Non-OAuth providers still passnulland are unaffected.createLocalVariablesInPlugin) slicesserverResolvedTokens?.[theme.id]at each iteration via adeltaFor()helper — every mode gets its own delta or falls back to local resolution.Same-class bug in the WITHOUT_MODES path
createVariablesFromSetsalso used the flat active-theme delta for every exported set. That would leak active-theme values into unrelated sets under OAuth. Fixed by passingnullfor the WITHOUT_MODES path — per-set exports have no theme dimension, so local resolution is authoritative there.Determinism and robustness (review round 2)
activeThemeis not layered in).nulland every mode falls back to local resolution. Prevents silent mixing of server-resolved and local values across modes.try/finallyaround fetch + dispatch so a throw can't strand the infiniteUI_CREATEVARIABLESspinner.projectIdmismatch after the fetch resolves — an in-flight project switch can no longer write A's deltas against B's Redux state.Tests
fetchServerResolvedTokensPerTheme.test.ts: target-group selections, ungrouped-theme fallback, ENABLED-set filter, all-or-nothing failure semantics.createLocalVariablesInPlugin.perThemeDelta.test.ts: real regression at the plugin-loop layer — asserts each per-theme call receives its own slice,nullfor absent themes, andnullfor anullmap.generateTokensToCreatetest kept but re-scoped honestly (leaf-level pass-through).Testing this change
Requires a Tokens Studio OAuth project with a theme group containing 2+ modes.
Automated coverage: