Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/remove node mutations #37

Merged
merged 8 commits into from
Apr 15, 2021
Merged

Conversation

dxinteractive
Copy link
Owner

@dxinteractive dxinteractive commented Apr 15, 2021

Nodes

  • Stop mutating child nodes Perf: stop relying on mutations within Nodes #23
  • Child nodes are always created upon attempted access, regardless of the existence of the chilren in the underlying data
  • Child nodes are not batch created upon parent access anymore, they are only created when the children themselves have attempted access
  • Changed expected behaviour - now if an object, map or basic type is removed, its node data and therefore information about its children is retained, not removed, so the user doesnt have to think about the depth of the change they are making (which is trickier to conceptualise with immer in the picture too). Arrays node data is removed as usual, because the dendriform-immer-patch-optimiser attempts to deal with child identification on arrays
  • Fixed bug where new nodes were needlessly created because immer sends an "add" instead of a "replace" referring to a path that already has a value on it. Unsure whether this is an immer issue or a dendriform one. This case is easily bandaided for now.

@dxinteractive dxinteractive force-pushed the feature/remove-node-mutations branch from b74e983 to 29fe490 Compare April 15, 2021 20:29
@dxinteractive dxinteractive merged commit 5082485 into master Apr 15, 2021
@dxinteractive dxinteractive deleted the feature/remove-node-mutations branch April 15, 2021 20:38
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.

1 participant