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
546 stars 24 forks source link

directRef? #166

Open mikecann opened 4 years ago

mikecann commented 4 years ago

When testing keystone models that have "ref" properties it can be tedious to have to setup a root model that contains the referenced model before testing the referencing model so I think some sort of "directRef" is useful.

Its basically just a customRef:

import { customRef, _Model } from "mobx-keystone";

// Not sure what the correct type should be here, 
// _Model doesnt seen to be correct
// so just leaving it as object for now
export const directRef = <T extends object>(obj: T) =>
  customRef<T>((obj as any).$modelType, {
    resolve(ref) {
      return obj;
    },
  })(obj);

And heres some tests explaining what I mean and how I like to use it:

import { Model, model, prop, Ref, rootRef } from "mobx-keystone";
import { directRef } from "../../src/modules/keystone";
import { computed } from "mobx";

@model("TestModelA")
class TestModelA extends Model({}) {
  @computed
  get id() {
    return this.$modelId;
  }
}

@model("TestModelB")
class TestModelB extends Model({
  modelA: prop<Ref<TestModelA>>(),
}) {
  @computed
  get modelAId() {
    return this.modelA.current.id;
  }
}

describe("directRef", () => {
  test(`root refs dont work`, () => {
    const a = new TestModelA({
      $modelId: "aaa",
    });

    const b1 = new TestModelB({
      modelA: rootRef<TestModelA>("TestModelA")("aaa"),
    });

    expect(() => b1.modelAId).toThrow(
      `a reference of type 'TestModelA' could not resolve an object with id 'aaa'`
    );

    const b2 = new TestModelB({
      modelA: rootRef<TestModelA>("TestModelA")(a),
    });

    expect(() => b2.modelAId).toThrow(
      `a reference of type 'TestModelA' could not resolve an object with id 'aaa'`
    );
  });

  it(`works`, () => {
    const a = new TestModelA({
      $modelId: "aaa",
    });

    const b1 = new TestModelB({
      modelA: directRef(a),
    });

    expect(b1.modelAId).toEqual(`aaa`);
  });
});

Thoughts? Is this a good idea? Is there a better way of doing it than this?

xaviergonz commented 4 years ago

Sorry for being late. So (if I get this right) basically you only want this for testing, so you can resolve references without a root model?

mikecann commented 4 years ago

Ye pretty much. I was sharing and checking I wasnt doing anything really dumb by doing this. If its not an issue you have encountered tho its okay ill close the issue :)

xaviergonz commented 4 years ago

Sounds useful. I'd change the name to something that makes its usage more obvious (e.g. testRef). Are you willing to create a PR?

mikecann commented 4 years ago

Well I dont want to add stuff to bloat the lib if im the only one likely to use it..

Also testRef, im less sure about that name as there is nothing inherently neccessary about it being in test code is there? It can just be used in normal code with multiple roots? Tho im not sure how snapshotting would work there..

Oh and yes I dont mind having a stab at a PR if you want, tho it might take a me a little while my new project is taking up a great deal of time at the mo :P