-
Notifications
You must be signed in to change notification settings - Fork 78
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
Add function statics and object callable properties #55
Merged
Merged
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
e291c1f
Add function statics and object callable properties
wgao19 3415f90
Update function statics and object callables
wgao19 a2208d3
Add TypeScript example for overloading callable
wgao19 faac61f
Updates on callable properties
wgao19 e103e1f
Updates on callable properties
wgao19 82f1b99
Add back the line # Read-only Types that got deleted by accident
wgao19 148fccb
Not sure how TS supports function statics
wgao19 6cc77ba
Resolve PR discussion and add function statics
wgao19 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 would be careful with this example. Here it might work, but this is not what actually happens at the type-level, and it might be confusing with real-world examples.
Flow will create multiple call signatures and intersect them, so the type of
{ (): string, (x: number): string }
is actually{(): string} & {(number): string}
, but we can simplify it to(() => string) & ((number) => string)
for the sake of the argument here.Flow doesn't support dependent types, so this can create a situation where we can describe overloaded implementations for functions that cannot be implemented using a single function body (from flow's perspective). Take this example:
We could change the interface to be
(number | string) => (number | string)
, but then the function consumer (which could be someone using your library) has to litter their code with useless checks (or flow-only code comments).Edit: typos and fluff
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.
Hmm I see. Thanks for sharing this! It’s a new perspective for me indeed, really appreciate it! 🧡💙🧡💙
Do you have a link to any real world example, e.g., a library you work on, so that I can see this in action and understand it more comprehensively?
Also, do you have links to any “further reading” that we can throw in as reference?
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.
No problem! These things are sadly not that well documented. I'm really glad you're putting effort into documenting this!
A few examples from the top of my head:
immer.js
uses it to overload theproduce
(default export) function which has multiple call signaturesstyled-components
uses it to allow being called with a string (react intrinsics), and to allow wrapping of componentsreact-redux
'sconnect
relies on overloading, too, depending on which arguments got passedre-reselect
uses it to overload thecreateCachedSelector
(alsore-select
).I can't think of anything right now. Most of my flow knowledge is kind of accumulated over time from keeping up with releases, understanding the flow code, and reading the commit messages.
Other than that, I think you're doing a great job of creating "further reading" materials yourself! :)
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.
Hey @omninonsense,
I've tried the example above and I could not make it work. Both Try Flow and TypeScript Playground are reporting roughly the same errors. It seems that they both are unable to consider the return type against their respective branches.
I've included a couple of simpler examples that exemplify the different syntax, as mentioned in your other comment.
Does it look good to you?
Thanks again for your inputs!
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.
Oh wow, thanks for sharing the libraries! Such a great learning material (massive chunks of overloaded call properties in re-select 😲
I see in styled-components libdef it does have the use case for overloaded branches with separate returns. Any idea why it's not working with our primitive examples?
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.
This is exactly why I added the example. 🙂 It's not possible to write overloaded function bodies. The best we can do is something like this:
This is not great since, but given the dynamic nature of javascript, this is probably the best it can get. We need to remember that Flow is just a superset of JS, not a compiler.
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.
The reason your example in the documentation worked is only because of the simple nature of the example and the fact that all signatures are compatible (since
(?number => number)
can be described as(() => number) & ((number) => number)
, which is what's happening here), but it might mislead someone reading the example that flow supports overloaded function implementations (when it only supports overloaded function declarations and overloaded call signatures).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.
Oh, I see 😀 Thanks for all the sharing, that's enlightening!
I guess we can get this merged now :)))))
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.
Regarding styled components: flow is smart enough to select the right overload declaration (at the type level) at call time based on arguments. It just doesn't have a way of writing overload implementations (because this is something JavaScript can't do either). The reason it supports overloaded declarations is because it makes it easier to describe APIs that way.
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.
Ah. Library definitions are a whole new field for me. Thanks for the pointer :) That was really helpful for me to understand the libdefs I'm using for Flow.