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

[Feature request]Should infer value from code block #24315

Closed
Albert-Gao opened this issue May 22, 2018 · 8 comments
Closed

[Feature request]Should infer value from code block #24315

Albert-Gao opened this issue May 22, 2018 · 8 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@Albert-Gao
Copy link

Albert-Gao commented May 22, 2018

interface IBox {
  row?: boolean,
  children: JSX.Element[] | JSX.Element,
}

const MyBox = (
  {row, children}: IBox
) => {
  let direction = 'column'
  if (row === true) direction = 'row'

  return (
    <View style={{
      flexDirection: direction,
    }}>
      {children}
    </View>
  )
};

Expected behavior:
It works

Actual behavior:
Now it will generate a very long and confusing error message. Why we need Property 'length' in type '{ flexDirection: string; }'. ?

Type '{ children: Element | Element[]; style: { flexDirection: string; }; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes & Readonly<{ children?: ReactNode; }> & Read...'.
Type '{ children: Element | Element[]; style: { flexDirection: string; }; }' is not assignable to type 'Readonly'.
Types of property 'style' are incompatible.
Type '{ flexDirection: string; }' is not assignable to type 'StyleProp'.
Type '{ flexDirection: string; }' is not assignable to type 'RecursiveArray<false | ViewStyle | RegisteredStyle>'.
Property 'length' is missing in type '{ flexDirection: string; }'.

How to make it work currently
Modify let direction = 'column' to let direction: 'column' | 'row' = 'column'

It's a very simple case, it should infer the value from the statement. Or at least, make that error message readable. In terms of type checking for React native, the error message are always like this. Seems not right, or I miss something here?

@s-ve
Copy link

s-ve commented May 22, 2018

Note that for this precise case, you can write :

const direction = row ? 'row' : 'column';

direction will be inferred as 'row' | 'column' as expected

@Albert-Gao
Copy link
Author

@s-ve Ah, I see. Thanks, silly me. Yes, this is more concise. Seems the pattern here is if it's an expression, it will get evaluated.

@ghost
Copy link

ghost commented May 22, 2018

So basically:

declare function f(p: "a" | "b"): void;

let x = "a";
if (something) x = "b";
f(x); // Error

TypeScript type inference tends to work forwards-only, so at let x = "a"; we need to decide a type for x right then -- we can't look over all references for x to try to infer a type as that would be slow. But in a const we have all the information we need up front, so it works.

@ghost ghost added the Design Limitation Constraints of the existing architecture prevent this from being fixed label May 22, 2018
@Albert-Gao
Copy link
Author

@Andy-MS Thanks for the detail.

So what about the error message?
Why it will display something like: Property 'length' is missing in type '{ flexDirection: string; }'.
What's the real error here?

A better error might be:

value of x has been changed, might not compatbile with type "a"|"b" for parameter p

The original error message is confusing and very long to interpreted.

@s-ve
Copy link

s-ve commented May 23, 2018

There is an existing suggestion that would make this error message a bit shorter : #24146.

I'm not sure why typescript decides to generate this error message here.

It would have been clearer if typescript chose to display something like type "string" is not assignable to "row" | "column" | "row-reverse" | "column-reverse".

Here is a shorter repro (Playground link) :

interface FlexStyle {
  flexDirection: 'column' | 'row'
}

const direction: string = 'column';
const x: FlexStyle | FlexStyle[] = {flexDirection: direction} // Error: Property 'length' is missing in type '{ flexDirection: string; }'.

@Albert-Gao Could you maybe fill a separate issue for this problem ?

@RyanCavanaugh
Copy link
Member

There are dozens of bugs suggesting we guess at what the "best" error message to issue is, please do not add another

@Albert-Gao
Copy link
Author

@s-ve Thanks, should close this one, and follow the #24146

@s-ve
Copy link

s-ve commented Jul 18, 2018

@Albert-Gao

The confusing error message issue (elaboration on FlexStyle[] instead of Flex) is tracked by #25750. There is a PR fixing this problem here,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

3 participants