TypeScript does not recognize overriding mandatory property with optional one in sub-model

[andreykurilin]

Minimal reproduction code

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

export const ErrorStore = types
  .model('ErrorStore', {
    value: '',
  })
  .actions((self) => ({
    set(value: string) {
      self.value = value;
    },
    reset() {
      self.value = '';
    },
  }))
  .views((self) => ({
    get formattedValue() {
      return self.value;
    },
  }));

export const NoDisplayError = ErrorStore.named('NoDisplayError').views(() => ({
  get formattedValue() {
    return undefined;
  },
}));


const InputStore = types.model("InputStore", {
    value: "",
    // developer should explicitly specify whether validation errors should be displayed or not
    error: types.union(ErrorStore, NoDisplayError)
});


const PasswordInputStore = InputStore.named("PasswordInputStore").props({
    // we have strict requirements for password, so validation errors should be displayed by default
    error: types.optional(types.union(ErrorStore, NoDisplayError), () => ErrorStore.create({}))
});


const passwordS = PasswordInputStore.create({});

Describe the expected behavior

Initialization of PasswordInputStore should not require specifying optional error property.

Describe the observed behavior

TypeScript fails as error property is not provided.

      TS2345: Argument of type '{}' is not assignable to parameter of type 'ModelCreationType<{ value: string | undefined; error: EmptyObject & Partial<{ value: string | undefined; }>; }>'.
  Property 'error' is missing in type '{}' but required in type '{ error: EmptyObject & Partial<{ value: string | undefined; }>; }'.

[coolsoftwaretyler]

Thanks for the issue report @andreykurilin, and I'm sorry for the inconvenience!

I see @thegedge has assigned this to himself, so I expect good things here.

I put together a CodeSandbox with the repro code in case that helps demonstrate the issue for others: https://codesandbox.io/p/sandbox/g3w8ft?file=%2Fsrc%2Findex.ts%3A8%2C15

[thegedge]

Sorry for the delay, but it's been really busy for me lately.

Thought I'd explore what's happening here:

Makes sense that if you have PropsA & PropsB with one having a required prop and the other not, the required prop wins. Since the way props works in MST is to "clobber" properties, I think the fix will be simple: Omit<PropsA, keyof PropsB> & PropsB.

[EDIT]
I suspect we may want to do this for anything else that "clobbers"; like views, actions, extend, and so on.

[coolsoftwaretyler]

Unfortunately, we'll need to revert this fix. The only way to model this correctly in TypeScript is running up against excessively deep type instantiation.

We are making a choice to serve the needs of users who are making many extensions, rather than users who need this optionality to be typed correctly.

At the moment, there's no viable solution to fix both bugs at the same time. Maybe if TypeScript makes progress on microsoft/TypeScript#34933, we can revisit it.

Another option for the long term will be fixing our type system overall. I don't have a clear roadmap or plan for that, but I will reopen this issue for now so this context doesn't get lost.

[mmakhalaf]

I'm getting a typescript compilation error which looks related to this fix.

This is the code to reproduce the error. I couldn't actually get the error in the sample unfortunately. In the real-world it was compiled as part of typescript project.

The compilation error in this case is in the form of TS2304: Cannot find name 'A' or A_1 coming from the line(s) you'll see in the .d.ts in the sample,

    }, Omit<{}, keyof A> & {

and

}, Omit<Omit<Omit<{}, keyof A_1> & {

Thanks!

[coolsoftwaretyler]

Hey @mmakhalaf - yeah, if the reproduction is happening intermittently, it may be due to the excessively deep instantiation. It's possible that a larger codebase is having problems with that type.

I'll deploy a fix to version 7.0.1 today. If you still have problems after that, please open a separate issue and we'll investigate further.

[mmakhalaf]

I'll deploy a fix to version 7.0.1 today. If you still have problems after that, please open a separate issue and we'll investigate further.

No errors using v7.0.1, thanks!