Skip to content
This repository has been archived by the owner on May 19, 2018. It is now read-only.

Fix arrow functions with destructuring, types & default value #94

Merged
merged 1 commit into from Aug 23, 2016

Conversation

danharper
Copy link
Member

e.g. var x = ({ a } : any = 'foo') => {}

Will currently throw: "Binding rvalue"

repl: Binding rvalue (1:9)
> 1 | var x = ({ a } : any = 'foo') => {}
    |          ^

Flow's toAssignable override wasn't calling the inner function after setting the type annotation, resulting in the node not having the type clarified from ObjectExpression to ObjectPattern (which would later cause it to throw as ObjectExpression isn't a valid lval).

Flow's "toAssignable" override wasn't calling the inner function,
resulting in the destructuring in an AssignmentPattern not having the
node type changed from ObjectExpression to ObjectPattern, resulting in
"Binding rvalue" thrown from "checkLVal()"
@codecov-io
Copy link

Current coverage is 96.91% (diff: 100%)

Merging #94 into master will not change coverage

@@             master        #94   diff @@
==========================================
  Files            19         19          
  Lines          2922       2922          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           2832       2832          
  Misses           90         90          
  Partials          0          0          

Powered by Codecov. Last update b649671...1f4b908

@danez
Copy link
Member

danez commented Aug 18, 2016

Good catch. Thanks for your contribution. lgtm

@@ -878,11 +878,11 @@ export default function (instance) {
});

instance.extend("toAssignable", function (inner) {
return function (node) {
return function (node, isBinding) {
Copy link
Member

@hzoo hzoo Aug 23, 2016

Choose a reason for hiding this comment

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

We can probably use ...args here and pass that down right? (although I see there's only 2 params)

works fine though

Copy link
Member Author

Choose a reason for hiding this comment

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

That's what I originally had tbh, then noticed toAssignableList was being explicit with the args.

Shall I change it?

Copy link
Member

Choose a reason for hiding this comment

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

Nah all good 😄

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

Successfully merging this pull request may close these issues.

None yet

4 participants