Convert setIn to SetNodeByDeltaPathVisitor #12820
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Contributor
✅ PR preview is ready!
|
This was referenced Oct 19, 2025
Collaborator
Author
This was referenced Oct 19, 2025
5cb8af0 to
f5842b6
Compare
1528c16 to
eeb90b9
Compare
9725ff2 to
9b06d11
Compare
Contributor
📉 Python coverage change detectedThe Python unit test coverage has decreased by 0.0000%
✅ Coverage change is within normal range. Coverage by files
|
Contributor
📈 Frontend coverage change detectedThe frontend unit test (vitest) coverage has increased by 0.0100%
✅ Coverage change is within normal range. |
223aaf7 to
9496462
Compare
9b06d11 to
17554c5
Compare
9496462 to
5cd1eaa
Compare
17554c5 to
4934ae1
Compare
807de2c to
4a7e699
Compare
5f44b36 to
4f7c717
Compare
d461b5e to
96fe649
Compare
4f7c717 to
3fc0e6d
Compare
96fe649 to
f582a47
Compare
db19c5d to
74be12a
Compare
f582a47 to
be4990a
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the setIn method from AppNode implementations (BlockNode and ElementNode) to use the visitor pattern via a new SetNodeByDeltaPathVisitor. The change removes the setIn method from the AppNode interface and individual node classes, replacing all usages with the visitor-based approach.
Key Changes
- Introduced
SetNodeByDeltaPathVisitorthat implements immutable node updates via the visitor pattern - Removed
setInmethod fromAppNodeinterface,BlockNode, andElementNodeclasses - Updated
AppRootto use the new visitor pattern through a newrunActionOnAllChildrenhelper method
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| SetNodeByDeltaPathVisitor.ts | New visitor implementation for setting nodes at delta paths with immutability |
| SetNodeByDeltaPathVisitor.test.ts | Comprehensive test suite covering visitor behavior, edge cases, and integration scenarios |
| ElementNode.ts | Removed setIn method that threw an error |
| BlockNode.ts | Removed setIn method implementation |
| BlockNode.test.ts | Updated tests to use SetNodeByDeltaPathVisitor.setNodeAtPath instead of direct setIn calls |
| AppRoot.ts | Added runActionOnAllChildren helper and updated three call sites to use new visitor pattern |
| AppNode.interface.ts | Removed setIn method from interface definition |
be4990a to
cd5b495
Compare
74be12a to
fbcb3ff
Compare
sfc-gh-bnisco
approved these changes
Nov 3, 2025
| deltaPath, | ||
| elementNode, | ||
| scriptRunId | ||
| ) as BlockNode, |
Collaborator
There was a problem hiding this comment.
suggestion: Consider making setNodeAtPath a generic function so that we aren't needlessly casting afterwards.
frontend/lib/src/render-tree/visitors/SetNodeByDeltaPathVisitor.test.ts
Outdated
Show resolved
Hide resolved
frontend/lib/src/render-tree/visitors/SetNodeByDeltaPathVisitor.ts
Outdated
Show resolved
Hide resolved
cd5b495 to
cbd79fb
Compare
fbcb3ff to
a1b1835
Compare
cbd79fb to
fe82e1a
Compare
ece38f4 to
e89016a
Compare
e89016a to
070a633
Compare
fe82e1a to
9d611de
Compare
070a633 to
5ea7844
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
The last of the visitors is the SetNodeByDeltaPathVisitor which sets the node in a hiearchy.
Testing Plan
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.