-
-
Notifications
You must be signed in to change notification settings - Fork 257
Optional names for function types and object type indexers #197
Conversation
Current coverage is 96.26% (diff: 100%)
|
@@ -1 +1,3 @@ | |||
* text eol=lf | |||
test/fixtures/es2015/uncategorised/22/actual.js binary | |||
test/fixtures/es2015/uncategorised/23/actual.js binary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was the issue you had with these two files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think they have weird characters that the .gitattribute
doesn't like if it treats them as text
$ git checkout master .gitattributes
$ git commit -m "revert gitattributes"
$ git rm --cached -r .
$ git reset --hard
$ g st
On branch flow
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git checkout -- <file>..." to discard changes in working directory)
modified: test/fixtures/esprima/invalid-syntax/migrated_0155/actual.js
modified: test/fixtures/esprima/invalid-syntax/migrated_0159/actual.js
no changes added to commit (use "git add" and/or "git commit -a")
$ g diff
warning: CRLF will be replaced by LF in test/fixtures/esprima/invalid-syntax/migrated_0155/actual.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in test/fixtures/esprima/invalid-syntax/migrated_0159/actual.js.
The file will have its original line endings in your working directory.
diff --git a/test/fixtures/esprima/invalid-syntax/migrated_0155/actual.js b/test/fixtures/esprima/invalid-syntax/migrated_0155/actual.js
index dd8e38b..ed49ef3 100644
--- a/test/fixtures/esprima/invalid-syntax/migrated_0155/actual.js
+++ b/test/fixtures/esprima/invalid-syntax/migrated_0155/actual.js
@@ -1,2 +1,2 @@
-//
+//
]
diff --git a/test/fixtures/esprima/invalid-syntax/migrated_0159/actual.js b/test/fixtures/esprima/invalid-syntax/migrated_0159/actual.js
index 583aa13..e85031c 100644
--- a/test/fixtures/esprima/invalid-syntax/migrated_0159/actual.js
+++ b/test/fixtures/esprima/invalid-syntax/migrated_0159/actual.js
@@ -1,2 +1,2 @@
-/*
+/*
*/]
@@ -529,6 +552,7 @@ pp.flowParsePrimaryType = function () { | |||
let tmp; | |||
let type; | |||
let isGroupedType = false; | |||
let oldNoAnonFunctionType = this.state.noAnonFunctionType; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add noAnonFunctionType
to the state so that v8 can better optimize the usage of the state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, will do!
@@ -0,0 +1 @@ | |||
var f = (x): number => 123 => 123; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to have a better error message that says something like return type annotation needs to be wrapped in parens or have named parameters
. But not sure if this is easy to accomplish.
This should not be a blocker, just if you have an idea on how this could be done easy, otherwise we can come back to that in a separate story.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's not easy to do. The problem is that the first =>
is parsed as arrow function's =>
. So we'd have to decide which parsing errors after the =>
would need this explanation and which are just normal arrow function parse errors.
Thanks for the PR, have just on micro performance change that would be nice to do, and 2 questions. |
Can this be merged now? |
Sorry I was on vacation and just got back, can merge tomorrow and try to get a release soon. |
In Flow v0.34.0, Flow will no longer require function type params and object type indexers to have names. So where you used to have to write
you can now write
Note: If you want to use a function type as the return type for an arrow function expression, you must either wrap it in parens or name each param: