Skip to content

fix incomplete theme application issue #3756

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
akshay-gupta7 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix incomplete theme application issue #3756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
022895c
fix incomplete theme application isue
akshay-gupta7 Jan 21, 2026
7122386
Merge branch 'main' into akshay/fix-incomplete-theme-application
akshay-gupta7 Jan 21, 2026
5390708
remove extra blank lines
akshay-gupta7 Jan 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/tokens-studio-for-figma/src/constants/ValidNodeTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ export const ValidNodeTypes: NodeType[] = [
'TEXT',
'VECTOR',
'STAR',
'SECTION',
];
13 changes: 8 additions & 5 deletions packages/tokens-studio-for-figma/src/plugin/NodeManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,15 @@ export class NodeManager {
for (let nodeIndex = 0; nodeIndex < relevantNodes.length; nodeIndex += 1) {
promises.add(defaultWorker.schedule(async () => {
const node = relevantNodes[nodeIndex];
const tokens = await tokensSharedDataHandler.getAll(node);

returnedNodes.push({
node: relevantNodes[nodeIndex],
tokens: await tokensSharedDataHandler.getAll(node),
id: node.id,
});
if (Object.keys(tokens).length > 0) {
returnedNodes.push({
node,
tokens,
id: node.id,
});
}
tracker.next();
tracker.reportIfNecessary();
}));
Expand Down
24 changes: 10 additions & 14 deletions packages/tokens-studio-for-figma/src/plugin/SharedDataHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,19 @@ class SharedDataHandler {
}

getAll<Result = string>(node: BaseNode) {
const keys = this.keys(node);
const result: Record<string, Result> = {};
keys.forEach((key) => {
if (key in Properties) {
const value = this.get(node, key);
if (value) {
try {
// make sure we catch JSON parse errors in case invalid property keys are set and found
// we're storing `none` as a string without quotes
const parsedValue = value === 'none' ? 'none' : JSON.parse(value);
result[key] = parsedValue;
} catch (err) {
console.warn(err);
}
Object.values(Properties).forEach((key) => {
const value = this.get(node, key);
if (value) {
try {
// make sure we catch JSON parse errors in case invalid property keys are set and found
// we're storing `none` as a string without quotes
const parsedValue = value === 'none' ? 'none' : JSON.parse(value);
result[key] = parsedValue;
} catch (err) {
console.warn(err);
}
}
return null;
});
Comment on lines +25 to 37
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

This refactoring changes the iteration logic from checking only the keys present on the node to checking all possible Properties enum values. While this ensures all known properties are checked (which may help with the incomplete theme application issue), it's less efficient as it makes up to 61 getSharedPluginData calls per node instead of only checking existing keys. Consider documenting why this change was necessary for fixing the theme application issue, as it's a significant performance trade-off.

Copilot uses AI. Check for mistakes.
return result;
}
Expand Down
1 change: 1 addition & 0 deletions packages/tokens-studio-for-figma/src/plugin/asyncMessageHandlers/update.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const update: AsyncMessageChannelHandlers[AsyncMessageTypes.UPDATE] = asy

allWithData = await defaultNodeManager.findBaseNodesWithData({
updateMode: msg.settings.updateMode,
nodesWithoutPluginData: true,
});

await updateNodes(allWithData, String(msg.settings.baseFontSize || 16));
Expand Down
2 changes: 1 addition & 1 deletion packages/tokens-studio-for-figma/src/plugin/updateNodes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export async function updateNodes(
try {
const rawTokenMap = destructureTokenForAlias(tokens, appliedTokens);
const tokenValues = mapValuesToTokens(tokens, appliedTokens);
setValuesOnNode(
await setValuesOnNode(
{
node,
values: tokenValues,
Expand Down
41 changes: 29 additions & 12 deletions packages/tokens-studio-for-figma/src/utils/findAll.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,40 @@
import { ValidNodeTypes } from '@/constants/ValidNodeTypes';

export function findAll(nodes: readonly BaseNode[], includeSelf = false, nodesWithoutPluginData = false): BaseNode[] {
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

The parameter name 'nodesWithoutPluginData' is misleading and creates confusion. When set to true, the function actually finds MORE nodes (including nodes without plugin data), not fewer. The name suggests it would exclude nodes with plugin data, but it actually enables a broader "Smart Discovery" mode that includes both nodes with plugin data AND instance children that may not have local plugin data. Consider renaming to something like 'includeInstanceChildren' or 'enableSmartDiscovery' to better reflect its actual behavior.

Copilot uses AI. Check for mistakes.
let allNodes = includeSelf ? [...nodes] : [];
const pluginDataOptions = nodesWithoutPluginData
? {}
: {
sharedPluginData: {
namespace: 'tokens',
},
};
const allNodesSet = new Set<BaseNode>(includeSelf ? nodes : []);

nodes.forEach((node) => {
if ('children' in node) {
allNodes = allNodes.concat(
if (nodesWithoutPluginData) {
// Smart Discovery:
// 1. Find all nodes with local token data (Figma optimized search)
const withLocalData = node.findAllWithCriteria({
types: ValidNodeTypes,
sharedPluginData: { namespace: 'tokens' },
});
withLocalData.forEach(n => allNodesSet.add(n));

// 2. Find all instances (their children have inherited token data)
const instances = node.findAllWithCriteria({
types: ['INSTANCE'],
});
instances.forEach(instance => {
allNodesSet.add(instance);
// For each instance, we must also check all its children
Comment on lines +17 to +23
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

The instance children search (lines 26-28) finds ALL children of ValidNodeTypes without filtering for plugin data. While this is necessary to capture instance children that inherit token data from their parent component (and thus have no local plugin data), it creates performance overhead by collecting many nodes that have no token data at all. These are later filtered out in NodeManager.ts (lines 83-89), but it would be more efficient to avoid collecting them in the first place. Consider adding a comment explaining why this broad search is necessary for the fix, as it's not immediately obvious that instance children need to be included even without local plugin data.

Suggested change
// 2. Find all instances (their children have inherited token data)
const instances = node.findAllWithCriteria({
types: ['INSTANCE'],
});
instances.forEach(instance => {
allNodesSet.add(instance);
// For each instance, we must also check all its children
// 2. Find all instances (their children may have inherited token data)
const instances = node.findAllWithCriteria({
types: ['INSTANCE'],
});
instances.forEach(instance => {
allNodesSet.add(instance);
// IMPORTANT:
// We intentionally search *all* children of instances, without filtering
// by shared plugin data here. Children can inherit token data from their
// parent component and therefore may not have any local plugin data set.
// These nodes are later filtered in NodeManager based on actual token data.
// Narrowing this search (e.g. by sharedPluginData) would cause us to miss
// inherited tokens on instance children.

Copilot uses AI. Check for mistakes.
instance.findAllWithCriteria({
types: ValidNodeTypes,
}).forEach(n => allNodesSet.add(n));
});
} else {
// Standard Discovery: Only nodes with local token data
node.findAllWithCriteria({
types: ValidNodeTypes,
...pluginDataOptions,
}),
);
sharedPluginData: { namespace: 'tokens' },
}).forEach(n => allNodesSet.add(n));
}
}
});

const allNodes = Array.from(allNodesSet);
return allNodes;
}
Loading