Skip to content

Conversation

@akshay-gupta7
Copy link
Contributor

Why does this PR exist?

Resolves #3408

What does this pull request do?

Testing this change

Additional Notes (if any)

@changeset-bot
Copy link

changeset-bot bot commented Jun 12, 2025

🦋 Changeset detected

Latest commit: c84c6ea

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
Copy link
Contributor

github-actions bot commented Jun 12, 2025

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

@github-actions
Copy link
Contributor

github-actions bot commented Jun 12, 2025

Commit SHA:909f2fbbb73a0987b56784fc41b5b40a86c07567
No changes to code coverage between the base branch and the head branch

@akshay-gupta7 akshay-gupta7 self-assigned this Jun 13, 2025
@akshay-gupta7 akshay-gupta7 requested a review from six7 June 16, 2025 07:23
themes: [],
activeTheme: {},
usedTokenSet: { core: TokenSetStatus.SOURCE, light: TokenSetStatus.ENABLED, theme: TokenSetStatus.ENABLED },
shouldUpdate: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

this only partially solves the issue.

there was a reason this isnt included in the setDefaultTokens call, and that is.. if you load tokens in a huge document and you'd have "apply to document" set, it means that on load of tokens it will run a document wide change. which can take quite a long time and isnt expected.

intead of just changing shouldUpdate to true - we should rather send an atomic update that just sets the new token values, and doesnt perform the whole other orchestration that comes through shouldUpdate: true

Copy link
Collaborator

Choose a reason for hiding this comment

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

you can reproduce this by:

  • load the default tokens
  • change a token from those (eg black to red)
  • create a rectangle and apply that token you just changed
  • create 1.000 replicas of that rectangle
  • change mode to "Apply to document"
  • Load the default tokens again.

Notice how it will - in this PR - make changes to aaaaaalll rectangles - without you ever requesting it. that's what shouldUpdate will do.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 17, 2025

Commit SHA:860a494fb8e05d6e92b146e696cf21b34bc23634
Current PR reduces the test coverage percentage by 1 for some tests

@akshay-gupta7 akshay-gupta7 requested a review from six7 June 18, 2025 06:58
@akshay-gupta7 akshay-gupta7 merged commit 3486fe7 into main Jul 7, 2025
14 checks passed
@akshay-gupta7 akshay-gupta7 deleted the fix-load-tokens branch July 7, 2025 13:39
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.

Tokens Lost if you don't click Apply To

3 participants