Any idea why a default argument might be undefined?

[maccman]

We are seeing edge cases sometimes where listView is sometimes undefined (see below)

@model('NoteStore')
export class NoteStore extends Model({
  listView: prop<NoteListView>(() => new NoteListView({})),
}) {

I'm wondering what could cause this. We do construct noteStores from snapshots, but we never include the listView in that snapshot.

[xaviergonz]

I tried this and all cases seemed to work (except when $modelType was not present and it was a prop and not a tProp)

https://stackblitz.com/edit/typescript-dxrdkn?file=index.ts

import { model, Model, prop, fromSnapshot, tProp } from 'mobx-keystone';

console.clear();

@model('NoteListView')
class NoteListView extends Model({}) {}

@model('NoteStore')
class NoteStore extends Model({
  listView: prop<NoteListView>(() => new NoteListView({})),
}) {}

{
  const m = new NoteStore({});
  console.log(m.listView);

  const m2 = new NoteStore({ listView: undefined });
  console.log(m2.listView);

  const m3 = fromSnapshot<NoteStore>({ $modelType: 'NoteStore' });
  console.log(m3.listView);

  const m4 = fromSnapshot<NoteStore>({
    $modelType: 'NoteStore',
    listView: undefined,
  });
  console.log(m4.listView);

  // this won't work because the model type is missing
  const m5 = fromSnapshot<NoteStore>({});
  console.log(m5.listView);
}

@model('NoteStore2')
class NoteStore2 extends Model({
  listView: tProp(NoteListView, () => new NoteListView({})),
}) {}

{
  const m = new NoteStore2({});
  console.log(m.listView);

  const m2 = new NoteStore2({ listView: undefined });
  console.log(m2.listView);

  const m3 = fromSnapshot(NoteStore2, { $modelType: 'NoteStore2' });
  console.log(m3.listView);

  const m4 = fromSnapshot(NoteStore2, {
    $modelType: 'NoteStore2',
    listView: undefined,
  });
  console.log(m4.listView);

  const m5 = fromSnapshot(NoteStore2, {});
  console.log(m5.listView);
}

Maybe you are not passing $modelType to fromSnapshot sometimes?

[maccman]

Yes it's quite bizarre. I'm going to try upgrading mobx-keystone.

[maccman]

Thank you for looking into it.

[maccman]

Also seeing this

[xaviergonz]

how was that object created?
do you have any action that might set this.volatileGraphs to undefined?

[maccman]

how was that object created?

Loading a snapshot:

do you have any action that might set this.volatileGraphs to undefined?

Nope, not possible.

[maccman]

Ah I'm using applySnapshot rather than fromSnapshot - would that change anything?

[maccman]

Repro: https://typescript-ccdhgu.stackblitz.io

[maccman]

I've got a reproduction of the issue here:

https://stackblitz.com/edit/typescript-ccdhgu?file=index.ts

It seems that when using applySnapshot, if you don't pass in all the properties then they get set to undefined.

[xaviergonz]

Ah, that's actually correct. applySnapshot is meant to update a model, not as a creation method (such as new or fromSnapshot), and default values are only applied on creation. If applySnapshot applied default values then the following case wouldn't be possible:

import { model, Model, prop, applySnapshot, getSnapshot } from 'mobx-keystone';

console.clear();

@model('GS')
export class GS extends Model(
  {
    n: prop<number | undefined>(1).withSetter(),
  },
  {}
) {}

const g1 = new GS({});
console.log(g1.n); // 1

const g2 = new GS({});
console.log(g2.n); // 1

g1.setN(undefined);
console.log(g1.n); // undefined

const g1sn = getSnapshot(g1);
console.log(g1sn.n); // undefined

applySnapshot(g2, g1sn);
// if applySnapshot were to set the default value here it would be 1, which is incorrect
console.log(g2.n); // undefined

[xaviergonz]

That being said maybe such a thing does not make much sense anyway... I'll give it a thought.

[xaviergonz]

Ok, in the end I changed my mind. It does not make sense to have applySnapshot put an undefined there when, later, either new or fromSnapshot will transform it into the default value anyway.

It is fixed in v0.69.5. Additionally, setting a prop that has a default value to null or undefined will now set the default value instead.

[xaviergonz]

@maccman

[maccman]

Fantastic! Thank you.