Make 'ASTDefinitionBuilder' responsible only for build types from AST #1230
Conversation
src/utilities/extendSchema.js
Outdated
| } | ||
| return type; | ||
| return (extendedType: any); |
There was a problem hiding this comment.
I suggest factoring this into two functions, getExtendedType and extendType, the first one performing caching (with a comment that extendType need only be called once per type). That should allow you to remove or at least further isolate the any cast.
There was a problem hiding this comment.
@leebyron Great idea 👍 I will make the change in a few minutes.
There was a problem hiding this comment.
There was a problem hiding this comment.
Cast through any internally while keeping the correct function definition, and we'll report it to the flow team
There was a problem hiding this comment.
@leebyron Done. Can you please review?
IvanGoncharov
force-pushed
the
astBuilder
branch
from
35f9c1d
to
ca2cdc7
Compare
src/utilities/extendSchema.js
Outdated
| } | ||
| return type; | ||
| // Workaround: Flow should figure out correct type, but it doesn't. | ||
| return (extendedType: any); |
There was a problem hiding this comment.
Rather than mutating a variable in each branch, just return the value in each branch. Flow can't figure out what the type of extendedType is, since there are so many different possible local mutations
There was a problem hiding this comment.
@leebyron I did that initially but it forces me to do typecast in every branch. Plus explaining workaround is more complicated.
Flow can't figure out what the type of extendedType is, since there are so many different possible local mutations
Flow has exactly the same problem figuring types both with or without mutations. You can see it in CI logs for your commit and in Flow playground:
|
@leebyron a8b9539 is failing flow check and I don't know how to fix it: #1230 (comment) |
|
Looks great! Thanks for iterating towards this better version |
It's part of #1199
Its primary purpose is to make 'ASTDefinitionBuilder' responsible only for build types from AST.
It's not only made possible to extract
transformSchemainto the separate function but also make the code more modular and simple.