Skip to content

Implement Node Manager#1004

Closed
camielvs wants to merge 1 commit into10-07-usenodemanager_hookfrom
09-29-implement_node_manager
Closed

Implement Node Manager#1004
camielvs wants to merge 1 commit into10-07-usenodemanager_hookfrom
09-29-implement_node_manager

Conversation

@camielvs
Copy link
Copy Markdown
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
Copy Markdown
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 279521f to 2c6d7b6 Compare October 9, 2025 19:19
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch 2 times, most recently from ba7a488 to fbccd60 Compare October 10, 2025 18:08
@camielvs camielvs mentioned this pull request Oct 10, 2025
3 tasks
@camielvs camielvs force-pushed the 10-07-usenodemanager_hook branch from 63a6e06 to e5d39ad Compare October 10, 2025 19:56
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch 3 times, most recently from a167f91 to a3cceee Compare October 10, 2025 20:47
@camielvs camielvs force-pushed the 10-07-usenodemanager_hook branch from e5d39ad to e42d6ea Compare October 14, 2025 19:56
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch 2 times, most recently from ab4936a to 86364a0 Compare October 14, 2025 20:01
@camielvs camielvs force-pushed the 10-07-usenodemanager_hook branch 2 times, most recently from dc68741 to 53aa077 Compare October 14, 2025 20:12
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch 2 times, most recently from 17e9d16 to 18e75de Compare October 14, 2025 22:34
@camielvs camielvs force-pushed the 10-07-usenodemanager_hook branch from 53aa077 to 3f1dd88 Compare October 14, 2025 22:34
@camielvs camielvs mentioned this pull request Oct 15, 2025
3 tasks
@camielvs camielvs force-pushed the 10-07-usenodemanager_hook branch from 3f1dd88 to e6bbf75 Compare October 15, 2025 18:09
@camielvs camielvs force-pushed the 09-29-implement_node_manager branch from 18e75de to 6d8f3be Compare October 15, 2025 18:09
@camielvs camielvs marked this pull request as ready for review October 15, 2025 18:39
@camielvs camielvs assigned maxy-shpfy and Mbeaulne and unassigned Mbeaulne and maxy-shpfy Oct 15, 2025
@camielvs camielvs requested a review from maxy-shpfy October 15, 2025 20:45
Copy link
Copy Markdown
Collaborator Author

Closed in favour of v2 Editor migration. See #2028

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.

3 participants