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

fixes for typescript 2.7 #667

Merged
merged 1 commit into from
Mar 7, 2018
Merged

fixes for typescript 2.7 #667

merged 1 commit into from
Mar 7, 2018

Conversation

xaviergonz
Copy link
Contributor

@xaviergonz xaviergonz commented Feb 18, 2018

Basic fixes for Typescript 2.7, tests passing.

The only problem seems to be with getSnapshot (and other stuff that uses ISnapshottable), which for new versions have to be manually typecasted to typeof Model.SnapshotType (e.g. getSnapshot<typeof Model.SnapshotType>(node))

This is basically because of this: microsoft/TypeScript#19513
ISnapshottable is basically now "merged" and lost into the new types, quoting:

When inferring multiple object literal types for a type parameter, the object literal types are normalized into a single inferred union type.

Since ISnapshottable is basically an empty interface it eventually gets lost. Probably there could be a way around this by adding some info that makes the interface unique, such as:

const snapshottableSymbol = Symbol();
export interface ISnapshottable {
  readonly [snapshottableSymbol]: true;
}

And while that made the type be applied again and not be lost, it couldn't be inferred on this check: export function getSnapshot<S>(target: ISnapshottable<S>): S, probably due to this:

When checking a type relationship between a source type S and a target type T, if T is an object literal type, any excess properties in S are required to have type undefined (normally such excess properties are permitted to have any type).

On the good side the changes don't change how old versions of TS behave.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants