-
Notifications
You must be signed in to change notification settings - Fork 0
Description
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:
- ReactFlowBridge sets
data.isExpanded: !container.collapsed(line 259 insrc/bridges/ReactFlowBridge.ts) - ContainerNode was checking
if (data.collapsed)which was alwaysundefined - 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 filesnode.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 (
isExpandedorcollapsed) - 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- UpdateReactFlowNodeDatainterfacesrc/bridges/ReactFlowBridge.ts- Ensure consistent property namesrc/render/ContainerNode.tsx- Already fixed to useisExpandedsrc/__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]: unknownis 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.