Skip to content

Type safety issue: Index signature in ReactFlowNodeData hides property naming inconsistency #61

@jhellerstein

Description

@jhellerstein

Problem

The ReactFlowNodeData interface has an index signature ([key: string]: unknown) that allows any property to be accessed without type errors. This led to a subtle bug where:

  1. ReactFlowBridge sets data.isExpanded: !container.collapsed (line 259 in src/bridges/ReactFlowBridge.ts)
  2. ContainerNode was checking if (data.collapsed) which was always undefined
  3. TypeScript didn't catch this mismatch because the index signature allows both properties

This caused the info button for collapsed containers to never render, because the conditional if (data.collapsed) was always false.

Current Workaround

Changed ContainerNode.tsx to derive isCollapsed = !data.isExpanded to match what ReactFlowBridge actually sets.

Inconsistent Usage Across Codebase

Tests and old code check both variations:

  • node.data.collapsed - found in multiple test files
  • node.data.isExpanded - what ReactFlowBridge actually sets

The interface technically defines collapsed?: boolean but this is NOT what gets set by the bridge.

Suggested Fix

Option 1: Remove index signature (preferred for type safety)

export interface ReactFlowNodeData {
  label: string;
  longLabel?: string;
  showingLongLabel?: boolean;
  nodeType: string;
  isExpanded?: boolean; // Standardize on this
  // Remove: collapsed?: boolean;
  containerChildren?: number;
  // ... other explicit properties
  // REMOVE: [key: string]: unknown;
}

Option 2: Keep index signature but standardize naming

  • Choose ONE property name (isExpanded or collapsed)
  • Update ALL usages in codebase:
    • ReactFlowBridge (producer)
    • ContainerNode, StandardNode (consumers)
    • All test files

Option 3: Support both for backwards compatibility

// In ReactFlowBridge, set both:
data: {
  isExpanded: !container.collapsed,
  collapsed: container.collapsed, // Deprecated, for backwards compat
  // ...
}

Search for affected code

# Find all places checking these properties
git grep "data\.collapsed" src/
git grep "data\.isExpanded" src/
git grep "node\.data\.collapsed" src/
git grep "node\.data\.isExpanded" src/

Files to Update

  • src/types/core.ts - Update ReactFlowNodeData interface
  • src/bridges/ReactFlowBridge.ts - Ensure consistent property name
  • src/render/ContainerNode.tsx - Already fixed to use isExpanded
  • src/__tests__/**/*.ts - Update all test assertions
  • Documentation - Update any API docs mentioning these properties

Benefits

  • ✅ Compile-time type safety catches property name mismatches
  • ✅ Prevents similar bugs in the future
  • ✅ Better IDE autocomplete and IntelliSense
  • ✅ Clearer API contract for component consumers

Related

This is part of the broader type safety improvements discussed in the v1.0.0 architecture rewrite. The index signature was likely added for flexibility but it undermines type safety.


💡 Note: The index signature [key: string]: unknown is a common anti-pattern in TypeScript that trades type safety for flexibility. Consider if this flexibility is truly needed, or if it's better to explicitly type all expected properties.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinggood first issueGood for newcomers

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions