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

Model instance maybe fields becoming TypeScript optional fields when included in a types.union #1525

Closed
3 tasks done
fruitraccoon opened this issue May 18, 2020 · 3 comments
Closed
3 tasks done
Labels
help/PR welcome Help/Pull request from contributors to fix the issue is welcome Typescript Issue related to Typescript typings

Comments

@fruitraccoon
Copy link
Contributor

Bug report

  • I've checked documentation and searched for existing issues
  • 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
https://codesandbox.io/s/wispy-meadow-z49x9

I've pinned the MST and Mobx versions in the reproduction, so that the issue can be reproduced if new versions affect the behaviour in any way (good or bad!).

Describe the expected behavior
That the TypeScript interface derived from Instance<typeof Model> would always "fit" an instance of that model.

Describe the observed behavior
When a child model has an a "maybe" field, and is then used within a union type, attempting to pass an instance of this model from the union field to a React prop typed as Instance<typeof Model> fails with "Property '...' is optional in type ... but required in type 'IModel'"

It's probably easiest to view the linked reproduction code.

I think what's happening is that when the model type is coming from within the union, "maybe" fields are becoming optional fields - but they are still mandatory fields on the derived interface. I can see that typescript reports the model type differently when it's come from the union vs. when it has not.

I realise this is a pretty specific use case, but I thought it worth raising. If there's any issue with the reproduction or more information is required, please let me know!

@alex-shamshurin
Copy link

alex-shamshurin commented Dec 7, 2020

Hallo, I have the same issue.

Property is optional in type but required in type ... For example - property in model is name: types.maybe(types.string) type and ts object which I'd like to assign to this model is name? string which cause incompatibility error. I have tried to use types.optional but it doesn't help.

At root level of model I tried this type:

export type Complete<T> = {
  [P in keyof Required<T>]: Pick<T, P> extends Required<Pick<T, P>>
    ? T[P]
    : T[P] | undefined;
};

and being applied to root model it helps to transform typescript optional to types.maybe, but suddenly it does not work for inner models and problem still exists.

    "mobx-state-tree": "~4.0.2",
    "typescript": "~4.1.2",

@coolsoftwaretyler
Copy link
Collaborator

Hey @fruitraccoon - sorry it's been so long since anyone got back to you here. Not sure if this is still a relevant issue. I have two thoughts:

  1. In the short term, you can provide your itemWithIssueModel variable with an explicit type annotation, like this:
const itemWithIssueModel = store.itemWithIssue === "anotherValue" ? null : store.itemWithIssue as IModel;

I forked your code sandbox here with that change: https://codesandbox.io/s/pensive-goodall-rzz2p6?file=/src/index.tsx

  1. In the long term, I think this is another issue with our overall TypeScript system, which we know is a longstanding and frustrating problem. I'm going to keep this issue open and tag it as such. We are trying to revamp MST a little bit, and TS is one of the big ticket items for us.

If you're interested in helping, we'd love to chat. If not, we'll still get to work on this in the coming months. Appreciate all your time reproducing and documenting the problem.

@coolsoftwaretyler coolsoftwaretyler added help/PR welcome Help/Pull request from contributors to fix the issue is welcome Typescript Issue related to Typescript typings labels Jun 30, 2023
@coolsoftwaretyler
Copy link
Collaborator

Hey folks, we finally got around to this. Huge thanks to @thegedge for resolving it in #2151.

This may end up being a breaking change, so keep an eye out for a preview release and RFC around versioning.

Thanks for hanging in there with us on this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help/PR welcome Help/Pull request from contributors to fix the issue is welcome Typescript Issue related to Typescript typings
Projects
None yet
Development

No branches or pull requests

3 participants