xaviergonz / mobx-keystone

A MobX powered state management solution based on data trees with first class support for Typescript, support for snapshots, patches and much more
https://mobx-keystone.js.org
MIT License
554 stars 25 forks source link

Unexpected value of context from detached node #513

Closed graphographer closed 1 year ago

graphographer commented 1 year ago

While fixing up some tests, I ran into this mysterious issue. For no particular reason, I assumed that a detached node should return the default value for a given context: someContext.get(detachedNode) === defaultValue;

Instead, it's returning a value that would only have been set by the previous test. I spent a lot of time making absolutely sure I was creating new nodes for each test and not sharing any other state. Is there some sort of context cleanup I need to be running after each test? Are contexts resolved by any manner other than by object identity? Or are the values of contexts simply not to be trusted for detached nodes?

I can provide more details about my code once we've agreed that there isn't something very obvious about how contexts work that I missed.

xaviergonz commented 1 year ago

As long as the context is set to one of the detached node's parents and not the node itself that is what should happen. This unit test shows that:

test("context is back to default when the parent providing the context is no longer there", () => {
  const ctx = createContext(1)

  const p = createP() // p = { p2: {...} }
  ctx.set(p, 2)

  expect(ctx.get(p)).toBe(2)
  const p2 = p.p2!
  expect(ctx.get(p2)).toBe(2)

  p.setP2(undefined)

  // now that p is no longer providing the context, it should be back to the default
  expect(ctx.get(p2)).toBe(1)
})

maybe you are setting the context value in the detached node itself? (so it is providing its own value)

graphographer commented 1 year ago

Thank you for your quick response. I'm afraid this seems to be an issue with our test tools, but more specifically with registering and unregistering root nodes before and after tests. If I have time to look more carefully or create a minimal reproduction, I will report back here. I am still rather vexed by the persistence of context data between tests which generate new keystone nodes each time -- so that it's clear to me the stale data is hiding out in the context, and I'd like to know exactly why so I can avoid it in the future. In the meantime, I was able to successfully resolve it.