Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add Optional Support For Multiple References to an Object#1088

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
BellCubeDev wants to merge9 commits intoimmerjs:main
base:main
Choose a base branch
Loading
fromBellCubeDev:main

Conversation

@BellCubeDev
Copy link

@BellCubeDevBellCubeDev commentedJan 10, 2024
edited
Loading

What This Does

Some state trees may need to reference an object more than once (such as the tree for myfomod library). In essence, we store existing drafts when an off-by-default Immer class configuration option is enabled. This should be a painless solution. Specifics are described below.

Implementation Details

  • TwoWeakMap are used to keep track of draft states and related data at different parts of the immerification process:
    • existingStateMap_ maps a given base object to the first draft state created for it. This state includes a reference to the revokable draft.
      • If a state is referenced multiple times, it will be given a newrevoke_() function that, once called the first time, calls the oldrevoke_() function. The result is that the finalrevoke_() must be called once for every requested draft before the Proxy is finally revoked. Since a proxy which has has itsrevoke_() method called should be considered revoked by all code paths, duplicate calls should not be an issue.
    • During finalization,encounteredObjects keeps track of objects we've finalized and doesn't traverse an object if it's already seen it. It prevents infinite recursion when circular references are present.
  • Introduced theextraParents_ property to theImmerBaseState interface. This property keeps track of additional values that would normally be attached toparent_ so that functionality relying on parent_ (such as marking the parent state as modified) is retained for objects with multiple parent objects
  • For Maps and Sets, a proxy is established between the state and DraftMap/DraftSet classes to handle multiple references to these native classes while preserving the idea of having one DraftSet per reference.
  • During finalization, there may be a case where an unmodified object contains a reference to a modified object as a child and, thus, even unmodified children must be finalized. All objects encountered during finalization must be drafted and traversed so we can catch references to objects which were never referenced in the recipe.
    • Frozen data structures are not guaranteed to be immune to this and therefore should be warned against.
  • To enable the feature, it is the same as other Immer class configuration options (such asuseStrictShallowCopy). That is, either specify it in the config object passed to the class's constructor OR call the relevant class method,setAllowMultiRefs()

Note

Because of the extra computation involved with checking every proxied object against a map and traversing every object in a tree, enabling multi-ref will have a significant performance impact—even on trees which contain no repeated references.

Tests

The file__tests__/multiref.ts contains a number of tests related to this multi-reference support implementation. Such tests seek to verify that:

  • Direct 1-to-1 circular references (which Immer does a sanity check against normally) should not throw an error when multi-ref is enabled
  • When the properties of multiple references are modified, all references are modified
    • A number of tests to verify this functionality for each archetype of object are provided.
  • Unmodified references to the same object are kept
  • The same copy is provided for every reference (new references are strictly equivalent [===] just as the references beforeproduce() would have been)

Additionally, I've addedallowMultiRefs into the base.js test matrix and adjusted tests as necessary to accommodate forallowMultiRefs' required functionality.

Tests are performed on all relevant object archetypes where applicable.

Outstanding Discussion TODOs

  • What to do regarding documentation
  • Add an error for when WeakMap isn't supported in the current environment? (supported in every noteworthy browser and server environment since late 2015)

Note

I have yet to compare the performance of stock Immer to the version of Immer created by my pull request withoutallowMultiRefs enabled. There may be no significant difference but work might be needed to address significant performance regressions should they exist.

Caution

Some tests are still failing whenallowMultiRefs is set totrue. That said, NO TESTS FAIL WHENallowMultiRefs IS SET TOfalse.

@BellCubeDevBellCubeDev marked this pull request as ready for reviewJanuary 10, 2024 00:12
@BellCubeDevBellCubeDev marked this pull request as draftJanuary 10, 2024 02:04
@BellCubeDevBellCubeDevforce-pushed themain branch 3 times, most recently fromeebb9e2 to83e87ffCompareFebruary 4, 2024 02:53
# What This DoesSome state trees may need to reference an object more than once (such as the tree for my [fomod](https://www.npmjs.com/package/fomod) library). In essence, we store existing drafts when an off-by-default Immer class configuration option is enabled. This should be a painless solution. Specifics are described below.## Implementation Details* Two `WeakMap` are used to keep track of draft states and related data at different parts of the immerification process:  * `existingStateMap_` maps a given base object to the first draft state created for it. This state includes a reference to the revokable draft.    * If a state is referenced multiple times, it will be given a new `revoke_()` function that, once called the first time, calls the old `revoke_()` function. The result is that the final `revoke_()` must be called once for every requested draft before the Proxy is finally revoked. Since a proxy which has has its `revoke_()` method called should be considered revoked by all code paths, duplicate calls should not be an issue.  * During finalization, `encounteredObjects` keeps track of objects we've finalized and doesn't traverse an object if it's already seen it. It prevents infinite recursion when circular references are present.* Introduced the `extraParents_` property to the `ImmerBaseState` interface. This keeps track of additional values that would normally be attached to `parent_` so that functionality such as marking the parent state as modified is retained for objects with multiple parent objects* For Maps and Sets, a proxy is established between the state and DraftMap/DraftSet classes to handle multiple references to these native classes while preserving the idea of having one DraftSet per reference.* For Sets, each child draft has a single symbol value set so that a copy is prepared. (discussion needed; see TODOs below)* During finalization, objects may have drafted children and, thus, even unmodified children are finalized in multi-ref mode* To enable the feature, it is the same as other Immer class configuration options (such as `useStrictShallowCopy`). That is, either specify it in the config object passed to the class's constructor OR call the relevant method, `setAllowMultiRefs()`> [!NOTE]> Because of the extra computation involved with checking every proxied object against a map and traversing every object in a tree, enabling multi-ref will have a significant performance impact—even on trees which contain no repeated references.# TestsThe file `__tests__/multiref.ts` contains a number of tests related to this multi-reference support implementation. Such tests seek to verify that:* Direct circular references (which Immer tests for normally) do not throw an error when multi-ref is enabled* When the properties of multiple references are modified, all references are modified* Unmodified references to the same object are kept* The same copy is provided for every reference (new references are strictly equivalent [`===`] just as the references before `produce()` would have been)Tests are performed on all relevant object archetypes where applicable.# Outstanding Discussion TODOs* [ ] What to do regarding documentation* [ ] Possible alternate solution for preparing copies for multi-reference DraftSet children* [ ] Add an error for when WeakMap isn't supported in the current environment? (supported in every noteworthy browser and server environment since late 2015)
@BellCubeDevBellCubeDev marked this pull request as ready for reviewMarch 19, 2024 02:38
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@BellCubeDev

[8]ページ先頭

©2009-2025 Movatter.jp