2.0: Make variable creation work across multiple files #2568

six7 · 2024-04-04T07:51:52Z

Fixes #2023

This PR enables creation of variables that reference other variables part of other files.
Also fixed an issue with variable creation from sets, where there'd be duplicate variables
Renamed the function that's responsible for creating variables based on sets to something more understandable

Had to comment out a few lines part of the syncVariables and syncStyles files - but didnt want to do the brunt work of removing those functions from the plugin in this PR. will do that in a followup.

Testing this change

Create tokens and try to create from a set - try it in multple sets as well (something that was skipped in prior PRs it seems)
Create themes and tokens that reference other tokens in other themes, and have those create in multiple files and notice references will work

CleanShot.2024-04-08.at.23.28.05.mp4

CleanShot.2024-04-08.at.23.31.25.mp4

changeset-bot · 2024-04-04T07:51:55Z

🦋 Changeset detected

Latest commit: 19d781f

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

…a-plugin into six7/remote-variables

packages/tokens-studio-for-figma/src/plugin/createLocalVariablesInPlugin.ts

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

six7 · 2024-04-08T07:02:36Z

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

          referenceVariableCandidates.push({
            variable,
            modeId: mode,
-            referenceVariable: referenceTokenName.split('.').join('/'),
+            referenceVariable: referenceTokenName,


Changed this so we're normalzing on dot notation as we do elsewhere

six7 · 2024-04-08T07:03:32Z

packages/tokens-studio-for-figma/src/plugin/updateVariablesToReference.ts

+  await Promise.all(referenceVariableCandidates.map(async (aliasVariable) => {
+    const referenceVariable = figmaVariables.get(aliasVariable.referenceVariable);
+    if (!referenceVariable) return;
+    const variable = await figma.variables.importVariableByKeyAsync(referenceVariable);


changed implementation to get variable remotely instead of just checking locally

six7 · 2024-04-08T07:04:57Z

packages/tokens-studio-for-figma/src/utils/alias/checkCanReferenceVariable.tsx

@@ -2,7 +2,6 @@ import { AliasRegex } from '@/constants/AliasRegex';
 import { UpdateTokenVariablePayload } from '@/types/payloads/UpdateTokenVariablePayload';
 import { SingleToken } from '@/types/tokens';

-export function checkCanReferenceVariable(payload: UpdateTokenVariablePayload | SingleToken<true, { path: string, variableId: string }> | null) {
-  if (!payload) return false;
+export function checkCanReferenceVariable(payload: UpdateTokenVariablePayload | SingleToken<true, { path: string, variableId: string }>) {


removed the unneccessary check if what we pass in even exists - lets do this outside

six7 · 2024-04-08T21:34:40Z

packages/tokens-studio-for-figma/src/plugin/createLocalVariablesInPlugin.ts

        } else {
-          const newCollection = figma.variables.createVariableCollection(theme.group ?? theme.name);


refactored this to re-use instead of duplicating everything

…riables

…esnt include source sets

packages/tokens-studio-for-figma/cypress/downloads/downloads.html

packages/tokens-studio-for-figma/src/plugin/findCollectionAndModeIdForTheme.ts

cuserox

I trust it works as expected, judging by the videos and the thorough extra tests 💪

.changeset/empty-geckos-roll.md

cuserox · 2024-05-23T09:45:12Z

packages/tokens-studio-for-figma/src/plugin/createLocalVariablesInPlugin.ts


 export type LocalVariableInfo = {
  collectionId: string;
  modeId: string;
  variableIds: Record<string, string>
 };
+// This function is used to create variables based on themes


This gets triggered by the above message -> CREATE_LOCAL_VARIABLES?

Would be good to have a list of message handlers + what they do and what message triggers it, not a blocker but just to see the overlaps and maybe refactoring opportunities!

Yes:

figma-plugin/packages/tokens-studio-for-figma/src/plugin/controller.ts

Line 64 in a002cc9

AsyncMessageChannel.PluginInstance.handle(AsyncMessageTypes.CREATE_LOCAL_VARIABLES, asyncHandlers.createLocalVariables);

Async message event handlers, in the plugin, are registered in controller.ts.

This is called in the UI, here:

figma-plugin/packages/tokens-studio-for-figma/src/app/store/useTokens.tsx

Lines 522 to 526 in a002cc9

}, async () => await AsyncMessageChannel.ReactInstance.message({

type: AsyncMessageTypes.CREATE_LOCAL_VARIABLES,

tokens: multiValueFilteredTokens,

settings,

}));

Agree - the variables flow in psrticular feels super repetitive. I tried to refactor parts in this PR but much more can be done here

cuserox · 2024-05-23T10:10:03Z

packages/tokens-studio-for-figma/src/plugin/createLocalVariablesInPlugin.ts

+      const allVariableObj = await updateVariables({
+        collection, mode: modeId, theme, tokens, settings,
+      });
+      if (Object.keys(allVariableObj.variableIds).length > 0) {


Would allVariableObj always have variableIds defined?

Yes, but it can be an empty array. I didnt work on this original implementation and i think we should improve this in a follow-up

packages/tokens-studio-for-figma/src/plugin/createLocalVariablesInPlugin.ts

packages/tokens-studio-for-figma/src/plugin/createNecessaryVariableCollections.ts

packages/tokens-studio-for-figma/src/plugin/setValuesOnVariable.test.ts

cuserox · 2024-05-23T10:37:53Z

packages/tokens-studio-for-figma/src/plugin/setValuesOnVariable.test.ts

Not necessary to make the changes now, but a lot of these tests could be rearranged and grouped follow the order of logic of each file, to see better what happy / sad paths are being covered.

I added a note in 7b05ae3 and added setValuesOnVariable as something to pay closer attention to during DX step where we mentioned we wanted to increase coverage of some of our core functions (this one has a few missing steps in logic)

packages/tokens-studio-for-figma/src/plugin/updateVariables.test.ts

macintoshhelper · 2024-05-23T13:40:13Z

packages/tokens-studio-for-figma/src/plugin/createLocalVariablesInPlugin.ts

+      // This could be because the user is in a free Figma plan and we werent able to create more than 1 mode
+      if (!collection || !modeId) return;
+
+      const allVariableObj = await updateVariables({


Having two different files called updateVariables is a little confusing.

controller.ts -> asyncHandlers.updateVariables – this is a different function to updateVariables which is called in createLocalVariablesInPlugin.ts.
controller.ts -> asyncHandlers.createLocalVariables
asyncMessageHandlers/createLocalVariables.ts -> createLocalVariablesInPlugin(...)
createLocalVariablesInPlugin.ts -> updateVariables

In a future refactor it could be worth reducing the number of functions re message handling and reuse more code. Moving between many different files/functions makes it harder to track the data flow.

💯 I agree. We should refactor this. I think we can get away with just 1 function that just does things a little bit different depending on if we're using themes or if we're using sets (we also have different props being passed in).

I've added this as one of the issues in the DX epic. #2792

Co-authored-by: Celia Usero Navarro <114073780+cuserox@users.noreply.github.com>

…a-plugin into six7/remote-variables

working commit

be30905

six7 added 9 commits April 8, 2024 08:54

working state

927b5db

self review 1

23492bf

revert removal of sync styles

f4047ec

Update useTokens.test.tsx

9da576f

revert async change

f16af9b

Merge branch 'six7/remote-variables' of github.com:tokens-studio/figm…

3b4c1f0

…a-plugin into six7/remote-variables

revert controller

d5efe81

add changeset

8758c32

update changes to include token sets creation

58ad58c

six7 changed the title ~~wip: remote variables~~ Make variable creation work across multiple files Apr 8, 2024

six7 commented Apr 8, 2024

View reviewed changes

six7 marked this pull request as ready for review April 8, 2024 21:39

fix tests

ea3fffa

six7 changed the title ~~Make variable creation work across multiple files~~ 2.0: Make variable creation work across multiple files Apr 8, 2024

six7 added 2 commits April 9, 2024 13:09

Merge branch 'release-139' into six7/remote-variables

533d313

Merge remote-tracking branch 'origin/release-139' into six7/remote-va…

08e1b52

…riables

six7 linked an issue Apr 11, 2024 that may be closed by this pull request

Add support for referencing non-local variables #2023

Closed

mackoj mentioned this pull request Apr 16, 2024

Inconsistencies in Colors Between Local Variables and Token Studio #2657

Open

six7 marked this pull request as draft April 26, 2024 08:35

six7 added 8 commits May 1, 2024 12:13

Merge remote-tracking branch 'origin/release-139' into six7/remote-va…

6851ae4

…riables

fix tests and naming

c858fbd

Merge remote-tracking branch 'origin/release-139' into six7/remote-va…

c8aed98

…riables

undo rename

a002cc9

adjust contents

71ffe43

extract getOrCreateVariableCollection

b141fda

move local variables fetch into merge fn

eace1f0

fix tests

9b14831

six7 marked this pull request as ready for review May 8, 2024 15:15

six7 marked this pull request as draft May 8, 2024 15:19

six7 added 7 commits May 13, 2024 10:34

update state

e5e2ab8

Merge remote-tracking branch 'origin/release-139' into six7/remote-va…

508fbd3

…riables

move to own fn

ed266eb

update and add tests

fc2e316

update logic for renaming collections and modes

22df785

add check to not proceed if no mode exists

43cd43d

fix tests

b0d7826

six7 marked this pull request as ready for review May 20, 2024 07:11

fix infinite load and create styles with variables if active theme do…

7738b94

…esnt include source sets

LukeFinch reviewed May 23, 2024

View reviewed changes

packages/tokens-studio-for-figma/cypress/downloads/downloads.html Outdated Show resolved Hide resolved

LukeFinch reviewed May 23, 2024

View reviewed changes

packages/tokens-studio-for-figma/src/plugin/findCollectionAndModeIdForTheme.ts Show resolved Hide resolved

cuserox approved these changes May 23, 2024

View reviewed changes

macintoshhelper reviewed May 23, 2024

View reviewed changes

six7 and others added 6 commits May 26, 2024 09:28

add cypress/downloads to gitignore

e7f375b

add check for undefined

eb07fb8

add clarifying comments

2f352d0

update to use simpler reduce

98bbe78

Apply suggestions from code review

3c72c7e

Co-authored-by: Celia Usero Navarro <114073780+cuserox@users.noreply.github.com>

Merge branch 'six7/remote-variables' of github.com:tokens-studio/figm…

24deb36

…a-plugin into six7/remote-variables

six7 mentioned this pull request May 26, 2024

Create style / variable name normalizer utility #2791

Open

six7 added 2 commits May 26, 2024 09:46

update test naming

fb70375

add note to refactor test file

7b05ae3

six7 mentioned this pull request May 26, 2024

Consolidate createLocalVariables functions into one #2792

Open

update tests

19d781f

six7 merged commit 890c5b2 into release-139 May 26, 2024
5 of 6 checks passed

six7 deleted the six7/remote-variables branch May 26, 2024 09:42

six7 added the 2.0-rc7 label Jun 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2.0: Make variable creation work across multiple files #2568

2.0: Make variable creation work across multiple files #2568

six7 commented Apr 4, 2024 •

edited

changeset-bot bot commented Apr 4, 2024 •

edited

six7 Apr 8, 2024

six7 Apr 8, 2024

six7 Apr 8, 2024

six7 Apr 8, 2024

cuserox left a comment

cuserox May 23, 2024

macintoshhelper May 23, 2024 •

edited

six7 May 23, 2024

cuserox May 23, 2024

six7 May 23, 2024

cuserox May 23, 2024

six7 May 26, 2024

macintoshhelper May 23, 2024

six7 May 26, 2024

		} else {
		const newCollection = figma.variables.createVariableCollection(theme.group ?? theme.name);

	}, async () => await AsyncMessageChannel.ReactInstance.message({
	type: AsyncMessageTypes.CREATE_LOCAL_VARIABLES,
	tokens: multiValueFilteredTokens,
	settings,
	}));

2.0: Make variable creation work across multiple files #2568

2.0: Make variable creation work across multiple files #2568

Conversation

six7 commented Apr 4, 2024 • edited

Testing this change

changeset-bot bot commented Apr 4, 2024 • edited

🦋 Changeset detected

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cuserox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macintoshhelper May 23, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

six7 commented Apr 4, 2024 •

edited

changeset-bot bot commented Apr 4, 2024 •

edited

macintoshhelper May 23, 2024 •

edited