Skip to content

fix(variables): display name rendering - #1915

Merged
icecrasher321 merged 3 commits into
stagingfrom
fix/variables-node-view
Nov 12, 2025
Merged

fix(variables): display name rendering#1915
icecrasher321 merged 3 commits into
stagingfrom
fix/variables-node-view

Conversation

@icecrasher321

Copy link
Copy Markdown
Collaborator

Summary

Variables preview in the node.

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercelBot commented Nov 12, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentPreviewCommentsUpdated (UTC)
docsSkippedSkippedNov 12, 2025 5:03am

@greptile-apps

Copy link
Copy Markdown
Contributor

Greptile Overview

Greptile Summary

This PR implements display name rendering for variable assignments in workflow nodes by hydrating variable IDs to their human-readable names.

Key Changes:

  • Added getVariableDisplayName helper to resolve variable names from IDs or use stored variableName
  • Modified SelectValue to use custom children for display (non-standard Radix UI pattern)
  • Added variablesDisplayValue logic in workflow-block.tsx to show variable names in node preview
  • Added variables-input to SELECTOR_TYPES_HYDRATION_REQUIRED array

Critical Issue Found:
The workflow-block.tsx implementation uses useVariablesStore.getState() inside useMemo, which creates a snapshot that doesn't subscribe to store updates. This will cause variable names to be stale and not update when variables are renamed or modified in the variables panel.

Other Concerns:
The approach of putting children inside SelectValue is non-standard for Radix UI - typically SelectValue automatically displays the text from the matching SelectItem. While this may work in some cases, it could lead to unexpected behavior.

Confidence Score: 2/5

  • This PR contains a critical reactivity bug that will cause stale data display
  • The score reflects a critical issue in workflow-block.tsx where getState() is used inside useMemo without proper store subscription, which will prevent the UI from updating when variables change. Additionally, the non-standard use of children in SelectValue may cause unexpected display behavior.
  • Pay close attention to workflow-block.tsx - the store subscription pattern needs to be fixed to ensure reactive updates

Important Files Changed

File Analysis

FilenameScoreOverview
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel-new/components/editor/components/sub-block/components/variables-input/variables-input.tsx3/5added getVariableDisplayName helper and custom children to SelectValue - approach may not work as expected with Radix UI standard behavior
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx2/5added variables display hydration logic with critical issue: uses getState() inside useMemo which won't subscribe to store updates, causing stale data

Sequence Diagram

sequenceDiagram
participant User
participant VariablesInput
participant Select
participant VariablesStore
participant WorkflowBlock
User->>VariablesInput: Select variable from dropdown
VariablesInput->>VariablesInput: getVariableDisplayName(assignment)
alt has variableName
VariablesInput-->>Select: Display variableName
else has variableId
VariablesInput->>VariablesStore: Find variable by ID
VariablesStore-->>VariablesInput: Return variable.name
VariablesInput-->>Select: Display variable.name
else no value
VariablesInput-->>Select: Display "Select a variable..."
end
User->>WorkflowBlock: View variable assignments in node
WorkflowBlock->>WorkflowBlock: Check if subBlock type is variables-input
WorkflowBlock->>VariablesStore: getState().variables (snapshot)
Note over WorkflowBlock,VariablesStore: Issue: getState() doesn't subscribe to updates
WorkflowBlock->>WorkflowBlock: Filter by workflowId
WorkflowBlock->>WorkflowBlock: Map assignments to display names
WorkflowBlock-->>User: Display formatted variable names
Loading

@greptile-appsgreptile-appsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

3 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@icecrasher321
icecrasher321 merged commit cbc98ab into stagingNov 12, 2025
9 checks passed
waleedlatif1 pushed a commit that referenced this pull request Nov 12, 2025
* fix(variables): display name rendering
* cleanup
* cleanup more code
@waleedlatif1
waleedlatif1 deleted the fix/variables-node-view branch November 12, 2025 15:39
waleedlatif1 pushed a commit that referenced this pull request Nov 12, 2025
* fix(variables): display name rendering
* cleanup
* cleanup more code
@waleedlatif1waleedlatif1 mentioned this pull request Nov 12, 2025
10 tasks
Sign up for freeto 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.

1 participant

@icecrasher321