Skip to content

Conversation

@HerringtonDarkholme
Copy link
Member

What kind of change does this PR introduce? (check at least one)

  • Other, please describe:
    improve flow type coverage to make isDef narrow nullable value to no-null value.

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

Other information:

Currently isDef cannot narrow nullable value, as tracked in facebook/flow#34. Howerver, we can use a special %checks comment to instruct flow inline the function.

To make this happen, we need first to type check utl/index.js. So some type definition changes are required. I have tried my best to keep changes constrained to type level. But still some changes influence runtime.

export let warn = noop
export let tip = noop
export let formatComponentName
export let formatComponentName: Function = (null: any) // work around flow check
Copy link
Member Author

Choose a reason for hiding this comment

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

This cast is necessary because we cannot persuade flow that formatComponentName is initialized before other code.

const props = {}
const propOptions = Ctor.options.props
if (isDef(propOptions)) {
propsData = propsData || {}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is needed because propsData is possibly null.

@yyx990803 yyx990803 merged commit bb7c543 into vuejs:dev Apr 29, 2017
@yyx990803
Copy link
Member

TIL about %checks!

@AndrewSouthpaw
Copy link

Also, doing /* %checks */! RubyMine doesn't like the syntax otherwise, I didn't know it works with the comment block, so this was a huge help!

@HerringtonDarkholme HerringtonDarkholme deleted the flow-type branch October 29, 2017 12:33
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.

3 participants