Skip to content

Fix to typedefs for Typescript 2.7 #637

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.

Sign up for GitHub

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

Closed
runningskull wants to merge 1 commit into from
Closed

Fix to typedefs for Typescript 2.7 #637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{
"private": true,
"dependencies": {
"lerna": "^2.4.0",
"cross-env": "^5.1.1",
"husky": "^0.13.4",
"prettier": "^1.4.4",
"lerna": "^2.4.0",
"lint-staged": "^3.6.1",
"cross-env":"^5.1.1"
"prettier": "^1.4.4"
},
"scripts": {
"clean": "lerna clean",
Expand Down
2 changes: 1 addition & 1 deletion packages/mobx-state-tree/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
"tape": "^4.6.0",
"tslib": "^1.7.1",
"tslint": "^3.15.1",
"typescript": "^2.4.2"
"typescript": "^2.7.1"
Copy link
Member

@Bnaya Bnaya Feb 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its a yarn project,
You'll need to upgrade via yarn for the lock file to be updated

Copy link
Author

@runningskull runningskull Feb 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(done in latest commit)

},
"peerDependencies": {
"mobx": "^3.1.15"
Expand Down
18 changes: 10 additions & 8 deletions packages/mobx-state-tree/src/types/complex-types/model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export class ModelType<S, T> extends ComplexType<S, T> implements IModelType<S,

props<SP, TP>(
properties: { [K in keyof TP]: IType<any, TP[K]> } & { [K in keyof SP]: IType<SP[K], any> }
): IModelType<S & SP, T & TP> {
): IModelType<S & SP & Snapshot<SP>, T & TP> {
Copy link
Author

@runningskull runningskull Feb 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solution from #635

Copy link
Member

@Bnaya Bnaya Feb 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/runningskull/mobx-state-tree/blob/0812fa2771659ef1e8819040d3b78266a130733c/packages/mobx-state-tree/src/types/complex-types/model.ts#L454-L463

I have no idea why,
But uncommenting the additional props declaration also fix this issue

Copy link
Author

@runningskull runningskull Feb 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(done in latest commit)

Copy link
Member

@Bnaya Bnaya Feb 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mweststrate wdys?

Copy link
Member

@mweststrate mweststrate Feb 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the impression that property types are broken completely with TS 2.7, are you guys experiencing the same? I mean even x: types.number get's infered to x: any on the IModelType. Maybe we should just wait for TS 2.8 where we can fix this properly? cc @mattiamanzati

Copy link
Member

@mweststrate mweststrate Feb 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the impression that property types are broken completely with TS 2.7, are you guys experiencing the same? I mean even x: types.number get's infered to x: any on the IModelType. Maybe we should just wait for TS 2.8 where we can fix this properly? cc @mattiamanzati

Copy link
Member

@Bnaya Bnaya Feb 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mweststrate
check this out
http://www.typescriptlang.org/play/#src=type%20IModelProperties%3CT%3E%20%3D%20%7B%20%5BK%20in%20keyof%20T%5D%3A%20T%5BK%5D%20%7D%0D%0Afunction%20inferer%3CT%3E(properties%3A%20IModelProperties%3CT%3E)%3A%20IModelProperties%3CT%3E%20%7B%0D%0A%20%20%20%20return%20properties%0D%0A%7D%0D%0A%0D%0A%2F%2F%20if%20you%20hover%20a%20you%20will%20see%20that%20prop1%20and%20prop2%20are%20inferred%20as%20any%0D%0Aconst%20a%20%3D%20inferer(%7B%0D%0A%20%20%20%20prop1%3A%20%22asd%22%2C%0D%0A%20%20%20%20prop2%3A%201%0D%0A%7D)%3B%0D%0A%0D%0Atype%20IMGOODTYPE%20%3D%20IModelProperties%3C%7B%20a%3A%20string%20%7D%3E%3B%0D%0A%2F%2F%20but%20here%2C%20the%20original%20type%20of%20prop1%20and%20prop2%20are%20restored%20%0D%0Atype%20imgoodTYPEEevenahasbadone%20%3D%20typeof%20a%3B

type IModelProperties<T> = { [K in keyof T]: T[K] };
function inferer<T>(properties: IModelProperties<T>): IModelProperties<T> {
  return properties;
}

// if you hover a you will see that prop1 and prop2 are:
// inferred as any on typescript 2.7.1
// inferred as string and number on typescript 2.6.2
const a = inferer({
  prop1: "asd",
  prop2: 1,
});

type IMGOODTYPE = IModelProperties<{ a: string }>;
// but here, the original type of prop1 and prop2 are restored,
// even in typescript 2.7.1
type imgoodTYPEEevenahasbadone = typeof a;

return this.cloneAndEnhance({ properties } as any)
}

Expand Down Expand Up @@ -457,7 +457,7 @@ export interface IModelType<S, T> extends IComplexType<S, T & IStateTreeNode> {
props: { [K in keyof TP]: IType<any, TP[K]> | TP[K] } &
{ [K in keyof SP]: IType<SP[K], any> | SP[K] }
): IModelType<S & Snapshot<SP>, T & TP>
//props<P>(props: IModelProperties<P>): IModelType<S & Snapshot<P>, T & P>
props<P>(props: IModelProperties<P>): IModelType<S & Snapshot<P>, T & P>
views<V extends Object>(fn: (self: T & IStateTreeNode) => V): IModelType<S, T & V>
actions<A extends { [name: string]: Function }>(
fn: (self: T & IStateTreeNode) => A
Expand All @@ -480,11 +480,12 @@ export type Snapshot<T> = {
[K in keyof T]?: Snapshot<T[K]> | any // Any because we cannot express conditional types yet, so this escape is needed for refs and such....
}

export function model<T = {}>(): IModelType<T | Snapshot<T>, T>
export function model<T = {}>(properties: IModelProperties<T>): IModelType<Snapshot<T>, T>
export function model<T = {}>(
name: string,
properties?: IModelProperties<T>
): IModelType<Snapshot<T>, T>
export function model<T = {}>(properties?: IModelProperties<T>): IModelType<Snapshot<T>, T>
properties: IModelProperties<T>
): IModelType<T | Snapshot<T>, T>
/**
* Creates a new model type by providing a name, properties, volatile state and actions.
*
Expand All @@ -493,10 +494,11 @@ export function model<T = {}>(properties?: IModelProperties<T>): IModelType<Snap
* @export
* @alias types.model
*/
export function model(...args: any[]) {
const name = typeof args[0] === "string" ? args.shift() : "AnonymousModel"
export function model<T = {}>(...args: any[]): IModelType<Snapshot<any>, any> {
const name = typeof args[0] === "string" ? args.shift() as string : "AnonymousModel"
const properties = args.shift() || {}
return new ModelType({ name, properties })
const config = { name: name as string, properties } as ModelTypeConfig
return new ModelType(config)
}

export function compose<T1, S1, T2, S2, T3, S3>(
Expand Down
4 changes: 2 additions & 2 deletions packages/mobx-state-tree/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ export function extendKeepGetter(a: any, ...b: any[]) {
for (let i = 0; i < b.length; i++) {
const current = b[i]
for (let key in current) {
const descriptor = Object.getOwnPropertyDescriptor(current, key)
if ("get" in descriptor) {
const descriptor = Object.getOwnPropertyDescriptor(current, key) || {}
if (descriptor && "get" in descriptor) {
Object.defineProperty(a, key, { ...descriptor, configurable: true })
continue
}
Expand Down
4 changes: 4 additions & 0 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10503,6 +10503,10 @@ typescript@^2.4.2:
version "2.5.3"
resolved "https://registry.yarnpkg.com/typescript/-/typescript-2.5.3.tgz#df3dcdc38f3beb800d4bc322646b04a3f6ca7f0d"

typescript@^2.7.1:
version "2.7.1"
resolved "https://registry.yarnpkg.com/typescript/-/typescript-2.7.1.tgz#bb3682c2c791ac90e7c6210b26478a8da085c359"

ua-parser-js@^0.7.9:
version "0.7.17"
resolved "https://registry.yarnpkg.com/ua-parser-js/-/ua-parser-js-0.7.17.tgz#e9ec5f9498b9ec910e7ae3ac626a805c4d09ecac"
Expand Down