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

snapshotProcessor modifies the type it wraps #1897

Open
3 tasks done
scytacki opened this issue Apr 9, 2022 · 1 comment
Open
3 tasks done

snapshotProcessor modifies the type it wraps #1897

scytacki opened this issue Apr 9, 2022 · 1 comment
Labels
bug Confirmed bug help/PR welcome Help/Pull request from contributors to fix the issue is welcome level: intermediate

Comments

@scytacki
Copy link
Contributor

scytacki commented Apr 9, 2022

Bug report

  • I've checked documentation and searched for existing issues
  • I've made sure my project is based on the latest MST version
  • Fork this code sandbox or another minimal reproduction.

Sandbox link or minimal reproduction code

    const Todo1 = types.model({ text: types.maybe(types.string) });
    const Todo2 = types.snapshotProcessor(Todo1, {
      preProcessor(sn) {
        return {text: "todo2 text"};
      }
    });
    const todo1a = Todo1.create();
    expect(todo1a.text).toBeUndefined();

    const todo2 =Todo2.create();
    expect(todo2.text).toBe("todo2 text");

    const todo1b = Todo1.create();
    // This is the unexpected behavior
    expect(todo1b.text).toBe("todo2 text");

Describe the expected behavior
types.snapshotProcessor should not modify the type it is wrapping. This expectation is based on the semantics of the API as well as the documentation.

Describe the observed behavior
When a model instance is created from a type returned by types.snapshotProcessor, the type being wrapped is modified. The create function of the type being wrapped is modified to behave the same as the create function of the type wrapping it.

Notes
I tracked this down to SnapshotProcessor._fixNode which is called by SnapshotProcessor.instantiate.
SnapshotProcessor._fixNode calls proxyNodeTypeMethods(node.type, this, "create").
Where node.type is the type being wrapped and this is the type created by snapshotProcessor.

I haven't looked further to figure out why/if this proxy of create is required.

@coolsoftwaretyler
Copy link
Collaborator

@scytacki - sorry it took so long for anyone to get back to you. I agree this is a bug! I'm going to label it as such, along with help wanted/intermediate. Not sure when folks will get around to it, but I think this should get fixed up when we can.

Thanks for the initial investigation. If you're interested in putting together a fix, that would be great. No worries if it's been a little too long and you're uninterested.

@coolsoftwaretyler coolsoftwaretyler added bug Confirmed bug help/PR welcome Help/Pull request from contributors to fix the issue is welcome level: intermediate labels Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug help/PR welcome Help/Pull request from contributors to fix the issue is welcome level: intermediate
Projects
None yet
Development

No branches or pull requests

2 participants