Skip to content

refactor: replace deepCopyFunction with structuredClone#136

Merged
willeastcott merged 2 commits intomainfrom
refactor/use-structuredClone
Mar 9, 2026
Merged

refactor: replace deepCopyFunction with structuredClone#136
willeastcott merged 2 commits intomainfrom
refactor/use-structuredClone

Conversation

@willeastcott
Copy link
Copy Markdown
Contributor

Summary

  • Replace custom deepCopyFunction in util.ts with the built-in structuredClone
  • Delete src/util.ts (it only contained deepCopyFunction)

structuredClone is supported in all modern browsers and Node 17+.

Test plan

  • Verify npm run build completes successfully
  • Verify context menus on nodes and edges still function correctly (the two call sites that used deepCopyFunction)

@willeastcott willeastcott force-pushed the refactor/use-structuredClone branch from 49e2e68 to 82a60b8 Compare March 9, 2026 12:43
@willeastcott willeastcott requested a review from Copilot March 9, 2026 12:43
@willeastcott willeastcott self-assigned this Mar 9, 2026
@willeastcott willeastcott added the enhancement New feature or request label Mar 9, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors context-menu item cloning by replacing the custom deepCopyFunction utility with the platform structuredClone, and removes the now-unused utility file.

Changes:

  • Replaced deepCopyFunction(...) with structuredClone(...) for edge/node context menu item cloning.
  • Removed src/util.ts and the corresponding README entry.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/util.ts Removes the custom deep copy helper that was previously used for menu item cloning.
src/index.ts Switches edge/node context menu item cloning to structuredClone and removes the old import.
README.md Removes util.ts from the source layout list.
Comments suppressed due to low confidence (2)

src/index.ts:323

  • Using structuredClone here can throw at runtime when edgeSchema.contextMenuItems contains non-cloneable values (notably callback functions like onSelect, which PCUI Menu items commonly use). The previous deepCopyFunction preserved function references, so this is a breaking behavior change. Consider switching to a cloning approach that preserves functions (e.g., shallow-clone items/arrays) or explicitly constrain/validate contextMenuItems to structured-cloneable data before cloning (and document the restriction).
            const contextMenuItems = structuredClone(edgeSchema.contextMenuItems).map((item: any) => {
                if (item.action === GRAPH_ACTIONS.DELETE_EDGE) {
                    item.onSelect = () => {
                        this._dispatchEvent(GRAPH_ACTIONS.DELETE_EDGE, { edgeId: edgeId, edge: this._graphData.get(`data.edges.${edgeId}`) });
                    };

src/index.ts:433

  • _initializeNodeContextMenuItems adds onSelect functions to each item, and it’s invoked both from Graph.createNode and again inside GraphViewNode.addContextMenu. With structuredClone, the second invocation will attempt to clone items that now include functions and will throw a DataCloneError, breaking node context menus. Fix by ensuring menu items are only initialized/cloned once (single call site), or by replacing structuredClone with a clone that tolerates function properties for these menu item objects.
        const contextMenuItems = structuredClone(items).map((item: any) => {
            if (item.action === GRAPH_ACTIONS.ADD_EDGE) {
                item.onSelect = () => this._createUnconnectedEdgeForNode(node, item.edgeType);
            }
            if (item.action === GRAPH_ACTIONS.DELETE_NODE) {
                item.onSelect = () => {
                    this._dispatchEvent(GRAPH_ACTIONS.DELETE_NODE, this._deleteNode(node.id));
                };
            }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread src/index.ts
structuredClone is supported natively in all modern browsers and Node 17+.
This removes the custom deep-copy utility and its source file.

Made-with: Cursor
@willeastcott willeastcott force-pushed the refactor/use-structuredClone branch from cde6fcb to 5d7958c Compare March 9, 2026 12:50
@willeastcott willeastcott merged commit e45ddab into main Mar 9, 2026
2 checks passed
@willeastcott willeastcott deleted the refactor/use-structuredClone branch March 9, 2026 12:52
willeastcott added a commit that referenced this pull request Apr 29, 2026
`Graph.createNode` was calling `_initializeNodeContextMenuItems` and then
passing the result to `view.addNodeContextMenu`, which (via
`GraphViewNode.addContextMenu`) calls `_initializeNodeContextMenuItems`
again. The second invocation runs `structuredClone` on items that now
contain `onSelect` function references attached during the first
invocation, causing:

  Failed to execute 'structuredClone' on 'Window':
  () => this._createUnconnectedEdgeForNode(node, item.edgeType)
  could not be cloned.

This regression was exposed by #136 (replacing `deepCopyFunction` with
`structuredClone`); the previous custom deep-copy silently preserved
function references on objects, masking the latent bug.

Pass the raw schema items straight to `view.addNodeContextMenu` so
initialization happens exactly once inside `GraphViewNode.addContextMenu`,
matching the existing flow used by `updateNodeType`.

Fixed #141

Made-with: Cursor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants