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

Unexpected error "Сannot finalize the creation of a node that is already dead" while applying snapshot #2136

Open
3 tasks done
dfilatov opened this issue Jan 30, 2024 · 18 comments
Labels
bug Confirmed bug docs or examples Documentation or examples related

Comments

@dfilatov
Copy link

Bug report

  • I've checked documentation and searched for existing issues and discussions
  • 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

import { types, applySnapshot } from 'mobx-state-tree';

const Model = types.array(
    types.model({
        b: types.string,
        c: types.optional(
            types.model({
                d: types.snapshotProcessor(
                    types.model({ e: '' }),
                    {
                        preProcessor(e: string) {
                            return { e };
                        },

                        postProcessor(snapshot) {
                            return snapshot.e;
                        }
                    }
                )
            }),
            { d: '' }
        )
    })
);

const m = Model.create([{ b: 'b' }]);

applySnapshot(m, []);

Describe the expected behavior

Should work without any errors

Describe the observed behavior

Error: [mobx-state-tree] assertion failed: cannot finalize the creation of a node that is already dead

@coolsoftwaretyler
Copy link
Collaborator

I think this may be a docs and TypeScript issue. You're calling preProcessor and postProcessor, but the types.snapshotProcessor actually wants preProcess and postProcess.

You can see it working here: https://codesandbox.io/p/sandbox/issue-2136-7hpyd2?file=%2Fsrc%2Findex.ts%3A21%2C3

But there's TypeScript errors on those actions.

Here's the working code for posterity:

import { types, applySnapshot } from "mobx-state-tree";

const Model = types.array(
  types.model({
    b: types.string,
    c: types.optional(
      types.model({
        d: types.snapshotProcessor(types.model({ e: "" }), {
          // TypeScript errors here, but I think that's incorrect
          preProcess(snapshot: { e: string }) {
            return { e: snapshot.e };
          },

         // TypeScript errors here, but I think that's incorrect
          postProcess(snapshot: { e: string }) {
            return snapshot.e;
          },
        }),
      }),
      { d: { e: "e" } }
    ),
  })
);

const m = Model.create([{ b: "b" }]);

applySnapshot(m, []);

I'll tag this, and please accept our apologies for the bug! We'll close this issue when we fix the types, but please let me know if I've missed anything else.

@coolsoftwaretyler coolsoftwaretyler added bug Confirmed bug level: easy Typescript Issue related to Typescript typings labels Feb 3, 2024
@dfilatov
Copy link
Author

dfilatov commented Feb 3, 2024

@coolsoftwaretyler
Copy link
Collaborator

Ah, my bad! I had a false positive, was just getting those errors to go away.

There is a documentation issue. We call it preProcess and postProcess here: https://mobx-state-tree.js.org/overview/hooks#snapshot-processing-hooks. That's what threw me and got me here.

I'll remove the TypeScript label here. I've been messing around with the code and I think this is coming from postProcessor. If you remove that action, the error goes away.

@coolsoftwaretyler coolsoftwaretyler added docs or examples Documentation or examples related and removed Typescript Issue related to Typescript typings level: easy labels Feb 3, 2024
@coolsoftwaretyler
Copy link
Collaborator

Seems to be related to applying the snapshot as an empty array. I can get the processed snapshot, modify it in place, and apply it correctly:

import { types, applySnapshot, getSnapshot } from "mobx-state-tree";

const Model = types.array(
  types.model({
    b: types.string,
    c: types.optional(
      types.model({
        d: types.snapshotProcessor(types.model({ e: "" }), {
          preProcessor(snapshot: { e: string }) {
            return { e: snapshot.e };
          },

          postProcessor(snapshot: { e: string }, node) {
            return {
              ...snapshot,
              e: "modified",
            };
          },
        }),
      }),
      { d: { e: "e" } }
    ),
  })
);

const m = Model.create([{ b: "b" }]);

const snapshot = getSnapshot(m);

console.log("snapshot", snapshot);

const modifiedSnapshot = [...snapshot];

console.log("modifiedSnapshot", modifiedSnapshot);

modifiedSnapshot[0].c.d.e = "modifiedAgain";

applySnapshot(m, modifiedSnapshot);

console.log("snapshot one last time", getSnapshot(m));

Updated example there:

https://codesandbox.io/p/sandbox/issue-2136-7hpyd2?file=%2Fsrc%2Findex.ts%3A41%2C1

But if you try to apply as an empty array, the error comes back.

@dfilatov
Copy link
Author

dfilatov commented Feb 3, 2024

Sorry, I can't open your example:
image

@coolsoftwaretyler
Copy link
Collaborator

Whoops, getting used to the new CodeSandbox tools. This should work: https://codesandbox.io/p/sandbox/issue-2136-7hpyd2?file=%2Fsrc%2Findex.ts%3A41%2C1

@coolsoftwaretyler
Copy link
Collaborator

I've got some pass-by-reference errors in there, but I gotta run at the moment so I can revisit this later. Sorry 'bout that!

@dfilatov
Copy link
Author

dfilatov commented Feb 3, 2024

It's not related with an empty array, following code throws the same error:

const Model = types.array(
      types.model({
        b: types.string,
        c: types.optional(
          types.model({
            d: types.snapshotProcessor(types.model({ e: '' }), {
              preProcessor(e: string) {
                return { e };
              },

              postProcessor(snapshot) {
                return snapshot.e;
              }
            })
          }),
          { d: 'e' }
        )
      })
    );

    const m = Model.create([{ b: 'b' }, { b: 'bb' }]);

    applySnapshot(m, [{ b: 'b' }]); // error

Interestingly, if I remove types.optional in the middle then everything works:

    const Model = types.array(
      types.model({
        b: types.string,
        c:
          types.model({
            d: types.snapshotProcessor(types.model({ e: '' }), {
              preProcessor(e: string) {
                return { e };
              },

              postProcessor(snapshot) {
                return snapshot.e;
              }
            })
          })
      })
    );

    const m = Model.create([{ b: 'b', c: { d: '' } }, { b: 'bb', c: { d: '' } }]);

    applySnapshot(m, [{ b: 'b', c: { d: '' } }]);

@coolsoftwaretyler
Copy link
Collaborator

Interesting. Maybe types.optional is having trouble instantiating the default value as a snapshotProcessor.

@coolsoftwaretyler
Copy link
Collaborator

Hopefully it's just this: #2137

Once we get a test in there I can merge and deploy a patch, probably early this week.

@dfilatov
Copy link
Author

dfilatov commented Feb 4, 2024

Unfortunately, no (
It just fixes import but exception will be thrown anyway, I've checked it.

@coolsoftwaretyler
Copy link
Collaborator

Ah dang. Do you have the full text of the exception with that patched? That might be helpful

@dfilatov
Copy link
Author

dfilatov commented Feb 5, 2024

Ah dang. Do you have the full text of the exception with that patched? That might be helpful

image

@coolsoftwaretyler
Copy link
Collaborator

Thank you! I'll see if I can get anyone to investigate. If not, I can at some point in the coming weeks. Would welcome a PR if you trace the error on your own as well. Apologies for the inconvenience!

@dfilatov
Copy link
Author

Hi, @coolsoftwaretyler! Is there any progress?

@coolsoftwaretyler
Copy link
Collaborator

Sorry, not yet! Haven't had anyone volunteer to work on it.

@BohdanPylypchuk
Copy link

@coolsoftwaretyler I have the same problem, it starts with 5.4.0

@coolsoftwaretyler
Copy link
Collaborator

Thanks @BohdanPylypchuk. Still haven't had anyone volunteer to work on it, we would welcome assistance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug docs or examples Documentation or examples related
Projects
None yet
Development

No branches or pull requests

3 participants