Skip to content

Conversation

@camielvs
Copy link
Collaborator

@camielvs camielvs commented Sep 29, 2025

Description

Replace all task/input/output -> node id conversions with the new NodeManager. In other words, the nodeIdUtils file has been replaced with the new NodeManager.

i.e. replace all instances of inputNameToNodeId, outputNameToNodeId, and taskIdToNodeId (and the reverse operations) with the NodeManager's getNodeId and getRefId methods (or getInputNodeId and getOutputNodeId via the useNodeManager hook). This also includes replacement of all methods dealing with Handle ids as well, and these have been brought into the node manager via getHandleNodeId and getHandleInfo.

inputNameToNodeId --> getInputNodeId
outputNameToNodeId --> getOutputNodeId
taskIdToNodeId --> getTaskNodeId

All nodes & handles will now have a uniquely assigned node id for use in ReactFlow and via the NodeManager this is mapped back to their respective id in the component spec. For handles they are mapped to the parent object they are one.

Also includes a few minor refactors such as removeEdge, to make things more human-readable.

Includes some AI-generated updates to tests.

Closes Shopify/oasis-frontend#261

Type of Change

  • Cleanup/Refactor
  • Improvement

Checklist

  • I have tested this does not break current pipelines / runs functionality
  • I have tested the changes on staging

Test Instructions

  1. Check that new & existing pipelines continue to function as expected with the updated node ID management
  • can add tasks and IO nodes
  • can connect stuff
  • can copy stuff
  • can export, import, edit
  • can execute pipelines
    etc
  1. You can use DEBUG_MODE to see if node ids are generating and updating uniquely

Copy link
Collaborator Author

camielvs commented Sep 29, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@camielvs camielvs mentioned this pull request Sep 29, 2025
3 tasks
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch from 62e4c7d to 8c27ed8 Compare September 29, 2025 22:31
@camielvs camielvs force-pushed the 09-19-add_node_manager_to_node_creation_methods branch from 8132fb5 to af581a3 Compare October 1, 2025 23:16
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch from 8c27ed8 to 46676ff Compare October 1, 2025 23:16
@camielvs camielvs force-pushed the 09-19-add_node_manager_to_node_creation_methods branch from af581a3 to 5b8b110 Compare October 7, 2025 22:53
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch from 46676ff to 9cab80e Compare October 7, 2025 22:53
@camielvs camielvs changed the title Implement Node Manager Implement Node Manager - Stage 2 Oct 7, 2025
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch from 9cab80e to 50cbbc2 Compare October 7, 2025 23:23
@camielvs camielvs force-pushed the 09-19-add_node_manager_to_node_creation_methods branch from 5b8b110 to c55eb6c Compare October 7, 2025 23:23
This was referenced Oct 7, 2025
@camielvs camielvs changed the base branch from 09-19-add_node_manager_to_node_creation_methods to graphite-base/1004 October 7, 2025 23:42
@camielvs camielvs force-pushed the graphite-base/1004 branch from c55eb6c to aef47ff Compare October 7, 2025 23:42
@camielvs camielvs changed the base branch from graphite-base/1004 to 10-07-usenodemanager_hook October 7, 2025 23:42
@camielvs camielvs changed the title Implement Node Manager - Stage 2 Implement Node Manager Oct 7, 2025
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch from 2f84d5f to 10473ce Compare October 8, 2025 00:38
@camielvs camielvs changed the base branch from 10-07-usenodemanager_hook to graphite-base/1004 October 8, 2025 00:55
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch from 10473ce to 8396c6f Compare October 8, 2025 00:55
@camielvs camielvs changed the base branch from graphite-base/1004 to 09-29-add_debug_mode_for_viewing_reactflow_node_ids October 8, 2025 00:55
@camielvs camielvs changed the base branch from 09-29-add_debug_mode_for_viewing_reactflow_node_ids to graphite-base/1004 October 8, 2025 01:06
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch from 8396c6f to 83ecf4a Compare October 8, 2025 01:06
@camielvs camielvs force-pushed the 10-07-usenodemanager_hook branch 2 times, most recently from 1910574 to 755167d Compare October 17, 2025 18:47
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch 2 times, most recently from 3afa0a3 to 6e7153a Compare October 17, 2025 21:41
@camielvs camielvs force-pushed the 10-07-usenodemanager_hook branch 2 times, most recently from 089c958 to b79c576 Compare October 17, 2025 21:42
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch 2 times, most recently from 0c17c58 to 80fe92b Compare October 22, 2025 20:18
@camielvs camielvs force-pushed the 10-07-usenodemanager_hook branch 2 times, most recently from 4dce064 to 846582d Compare October 22, 2025 22:19
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch 2 times, most recently from 4ebe9af to a771300 Compare October 23, 2025 16:35
@camielvs camielvs force-pushed the 10-07-usenodemanager_hook branch from 1c05e06 to 588f464 Compare October 23, 2025 20:05
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch 2 times, most recently from b0d1e49 to 2cb7d00 Compare October 23, 2025 20:11
@camielvs camielvs force-pushed the 10-07-usenodemanager_hook branch from 588f464 to 36aac5b Compare October 23, 2025 20:11
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch from 2cb7d00 to 8e578be Compare October 23, 2025 23:23
@camielvs camielvs force-pushed the 10-07-usenodemanager_hook branch from 36aac5b to 70a3943 Compare October 23, 2025 23:23
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch from 8e578be to 9818be3 Compare October 24, 2025 00:15
@camielvs camielvs force-pushed the 10-07-usenodemanager_hook branch from 70a3943 to 6a50c55 Compare October 24, 2025 00:15
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch from 9818be3 to 705d37e Compare October 28, 2025 21:04
@camielvs camielvs mentioned this pull request Oct 28, 2025
4 tasks
@maxy-shpfy maxy-shpfy marked this pull request as draft October 30, 2025 16:55
@camielvs camielvs force-pushed the 10-07-usenodemanager_hook branch from 07c7df4 to beded30 Compare October 30, 2025 19:10
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch 2 times, most recently from 856c31c to d672fba Compare October 30, 2025 19:44
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch from d672fba to a8814d2 Compare October 31, 2025 00:23
@camielvs camielvs force-pushed the 10-07-usenodemanager_hook branch from 59a4d8f to 55384a0 Compare October 31, 2025 00:23
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.

4 participants