react-native typings not work since typescript 3.1.1

[vovkasm]

TypeScript Version: 3.2.0-dev.20180927

Search Terms: react-native, stylesheet

Code
I'm created repo to reproduce:

1. git clone https://github.com/vovkasm/rn-ts-3.1.1.git
2. cd rn-ts-3.1.1
3. npm install && npm test

Paste here for easy reading

import React from 'react'
import { StyleSheet, Text } from 'react-native'

const s = StyleSheet.create({
  didNotWork: {
    fontSize: 16,
    fontWeight: '900', // if we comment this line, errors gone
    marginTop: 5, // if this line commented, errors also gone
  },
  work: {
    fontSize: 18,
//    fontWeight: '900', // when uncommented also work
  },
})

export const sample1 = <Text style={s.work} />
export const sample2 = <Text style={s.didNotWork} />
// ^ this line generate error:
// index.tsx:17:30 - error TS2322: Type 'RegisteredStyle<{ fontSize: number; fontWeight: string; marginTop: number; }>' is not assignable to type 'StyleProp<TextStyle>'.
//   Type 'RegisteredStyle<{ fontSize: number; fontWeight: string; marginTop: number; }>' is not assignable to type 'RecursiveArray<false | TextStyle | RegisteredStyle<TextStyle> | null | undefined>'.
//     Property 'length' is missing in type 'Number & { __registeredStyleBrand: { fontSize: number; fontWeight: string; marginTop: number; }; }'.
// 17 export const sample2 = <Text style={s.didNotWork} />
//                                 ~~~~~
//   node_modules/@types/react-native/index.d.ts:907:5
//     907     style?: StyleProp<TextStyle>;
//             ~~~~~
//     The expected type comes from property 'style' which is declared here on type 'IntrinsicAttributes & IntrinsicClassAttributes<Text> & Readonly<{ children?: ReactNode; }> & Readonly<TextProps>'
export const sample3 = <Text style={{fontSize: 16, fontWeight: '900', marginTop: 5}} />

Expected behavior:
No errors

Actual behavior:
An error occured:

index.tsx:17:30 - error TS2322: Type 'RegisteredStyle<{ fontSize: number; fontWeight: string; marginTop: number; }>' is not assignable to type 'StyleProp<TextStyle>'.
  Type 'RegisteredStyle<{ fontSize: number; fontWeight: string; marginTop: number; }>' is not assignable to type 'RecursiveArray<false | TextStyle | RegisteredStyle<TextStyle> | null | undefined>'.
    Property 'length' is missing in type 'Number & { __registeredStyleBrand: { fontSize: number; fontWeight: string; marginTop: number; }; }'.

17 export const sample2 = <Text style={s.didNotWork} />
                                ~~~~~

  node_modules/@types/react-native/index.d.ts:907:5
    907     style?: StyleProp<TextStyle>;
            ~~~~~
    The expected type comes from property 'style' which is declared here on type 'IntrinsicAttributes & IntrinsicClassAttributes<Text> & Readonly<{ children?: ReactNode; }> & Readonly<TextProps>'

Playground Link: https://github.com/vovkasm/rn-ts-3.1.1

Related Issues: no

Sorry, I can't found a way to reduce this regression farther to smallest possible example. But it looks like this actually typescript regression because:

1. It works with ts 3.0.3
2. It magically (from my point of view) will pass test if we change code in unrelated parts (comment/uncomment some styles - see comments in index.tsx)
3. I traced type definitions of react-native.d.ts and they appears to be correct (again, from my point of view)...

[mattmccutchen]

I don't understand what's going on in the type inference on the call to StyleSheet.create. Somehow including both fontWeight and marginTop is causing it to behave in a way the @types/react-native authors didn't intend so that the type of the fontWeight property gets widened to string.

The rest I do understand: after the StyleSheet.create call, s.didNotWork is a RegisteredStyle<T> object where T is a type that is incompatible with TextStyle because the fontWeight got widened. This RegisteredStyle<T> was assignable to an unregistered TextStyle in TypeScript 3.0.3, but it no longer is because of #26790, which changed the weak type assignability check to apply when the source is a primitive type (as RegisteredStyle<T> is). #26790 was described as an "optimization" and appears to be more than that, but the change still seems reasonable to me, so I believe the fix will have to be elsewhere (in TypeScript or @types/react-native).

[mattmccutchen]

Reduced example (same behavior in 3.0.3 and 3.2.0-dev.20180927):

interface TextStyle extends ViewStyle {
  fontWeight?: "900";
}
interface ViewStyle {
  marginTop?: number;
}

declare function create<T extends { [P in keyof T]: TextStyle | ViewStyle }>(styles: T): T;
const s = create({
  didNotWork: {
    fontWeight: '900', // if we comment this line, errors gone
    marginTop: 5, // if this line commented, errors also gone
  },
});
const f1: TextStyle = s.didNotWork;

[ahejlsberg]

This is working as intended, but it's subtle to say the least. I'm going to describe what happens using the reduced example posted by @mattmccutchen above.

First, because the constraint for T is written in terms of T itself and we can't simultaneously depend on T and infer for T, we end up having no contextual type for the styles: T parameter. Therefore, we infer string and number instead of literal types for the fontWeight and marginTop properties. So, we end up with { didNotWork: { fontWeight: string, marginTop: number } } as the inference for T.

We then check for assignability to the instantiated constraint, i.e. { didNotWork: TextStyle | ViewStyle }. This check ends up succeeding because the inferred type is assignable to ViewStyle. However, the inferred type is not assignable to TextStyle so we get an error in the declaration of f1.

If you comment out fontWeight: '900' we proceed the same way as above and get the same error. However, if you comment out marginTop: 5 we now fail the constraint check during inference for T and default to the constraint itself, i.e. { fontWeight: TextStyle | ViewStyle }. We subsequently pass the signature applicability check because now the object literal has a contextual type with literal types in it.

As I said, it's subtle!

I'd suggest changing the constraint for T to use a string index signature:

declare function create<T extends { [x: string]: TextStyle | ViewStyle }>(styles: T): T;

With that change it works as expected because everything gets properly contextually typed.

[vovkasm]

@ahejlsberg
Sorry for disturb, but I need some clarification. I think I understand what was written. But then I supposed that this fix should not detect errors when we use not existing keys on T (because T extends generic object that can have any property [x: string]: TextStyle | ViewStyle).

I modified reduced example to check this:

interface TextStyle extends ViewStyle { fontWeight?: "900"; }
interface ViewStyle { marginTop?: number; }
declare function create<T extends { [P: string]: TextStyle | ViewStyle }>(styles: T): T

const s = create({
  didNotWork: {
    fontWeight: '900', // if we comment this line, errors gone
    marginTop: 5, // if this line commented, errors also gone
  },
})
const f1: TextStyle = s.didNotWork
const f2: ViewStyle = s.notExists
// ^--- Property 'notExists' does not exist on type '{ didNotWork: { fontWeight: "900"; marginTop: number; }; }'.

It is good that last line emit error in context of recommended fix. But isn't it counter intuitive? I don't understand this.

[fiznool]

If the behaviour is as expected, is it reasonable to assume that the React Native typings need to be updated to eliminate this discrepancy?

Until this is fixed I guess the workaround is to use TS 3.0.3 if you want to use @types/react-native?

[mattmccutchen]

But then I supposed that this fix should not detect errors when we use not existing keys on T (because T extends generic object that can have any property [x: string]: TextStyle | ViewStyle).

Here T would be an object literal type such as {didNotWork: {fontWeight: "900", marginTop: number}}, which has no index signature, so trying to access a nonexistent property generates an error. The syntax T extends { [P: string]: TextStyle | ViewStyle } really just means T is assignable to { [P: string]: TextStyle | ViewStyle }. Arguably the word "extends" is a misnomer here given that the assignability relation supports several things that are not in the spirit of "extending", including (1) letting an object literal type be assignable to a type with an index signature (the relevant rule in our case) and (2) removing optional properties.

[vovkasm]

@mattmccutchen Thank you for explanation!

Then this definitely can be fixed on typings side :-)

[vovkasm]

Can I close this issue now? It seems no work can be done on this side...

[mattmccutchen]

Can I close this issue now?

Yes please. Otherwise, the TypeScript team has an automated process that will eventually close it based on the "Working as Intended" label.

[ScreamZ]

@vovkasm So what now ?

[vovkasm]

@ScreamZ I don't know... most probably we will not be able express such types reliable until typescript 10 or 20 :-/
Changes suggested by typescript team simple did not work...

[ScreamZ]

@vovkasm GeekyAnts/NativeBase#2123 Looks like the same with native-base

[ahejlsberg]

I see the fix to react-native got stalled with the fix I suggested (because it adds a new requirement for arguments to have a string index signature). Let me suggest a different fix. Change react-native's StyleSheet.create function in index.d.ts to have the following declaration:

export function create<T extends NamedStyles<T> | NamedStyles<any>>(styles: T): { [P in keyof T]: RegisteredStyle<T[P]> };

The NamedStyles<any> part of the union type causes the constraint to have a string index signature (such that contextual types are provided), whereas the NamedStyles<T> part of the constraint ensures that a final inferred type without a string index signature is accepted.

Best I can tell this fixes the issue and doesn't require any other changes.

[sublimeye]

@ahejlsberg Could you provide an example of how exactly do you override create?

[attilah]

In the same area of React Native I found a similar error, after the fix was applied and I think it can be a TypeScript error. I've extracted the relevant parts so the repro with the latest TSC is easy.

Code:

interface FlexStyle {
    width?: number;
}

interface ViewStyle extends FlexStyle {
    overflow?: "visible" | "hidden" | "scroll";
}

interface ImageStyle extends FlexStyle {
    overflow?: "visible" | "hidden";
}

type NamedStyles<T> = { [P in keyof T]: ViewStyle | ImageStyle };

type RegisteredStyle<T> = number & { __registeredStyleBrand: T };

type Foo = {
    bar: RegisteredStyle<ImageStyle>;
};

function create<T extends NamedStyles<T> | NamedStyles<any>>(styles: T): { [P in keyof T]: RegisteredStyle<T[P]> }
{
    return null;
}

const fooStyles: Foo = create<NamedStyles<Foo>>({
    bar: {
        width: undefined
    }
});

After calling TSC test.ts, this is the error:

test.ts:26:7 - error TS2322: Type '{ bar: RegisteredStyle<ViewStyle | ImageStyle>; }' is not assignable to type 'Foo'.
  Types of property 'bar' are incompatible.
    Type 'RegisteredStyle<ViewStyle | ImageStyle>' is not assignable to type 'RegisteredStyle<ImageStyle>'.
      Type 'RegisteredStyle<ViewStyle | ImageStyle>' is not assignable to type '{ __registeredStyleBrand: ImageStyle; }'.
        Types of property '__registeredStyleBrand' are incompatible.
          Type 'ViewStyle | ImageStyle' is not assignable to type 'ImageStyle'.
            Type 'ViewStyle' is not assignable to type 'ImageStyle'.
              Types of property 'overflow' are incompatible.
                Type '"hidden" | "visible" | "scroll"' is not assignable to type '"hidden" | "visible"'.
                  Type '"scroll"' is not assignable to type '"hidden" | "visible"'.

26 const fooStyles: Foo = create<NamedStyles<Foo>>({

overflow defined in 2 different interfaces and I think that TS compiler fails early on mismatching valuesets because the interface types are not related, but one of them could be the result of the create method call.

That is just the top of the cake that in React Native's case, FlexStyle has the overflow property, so an the ImageStyle interface which extends FlexStyle "overwrites" the valueset, but this time I separated it for clarity.