-
Notifications
You must be signed in to change notification settings - Fork 149
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
Export type declarations for type-checking with Flow #180
Open
hallettj
wants to merge
15
commits into
swannodette:master
Choose a base branch
from
hallettj:feature/flow-types
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
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
Hand-written definitions for Mori types are in mori.js.flow. In projects that use Flow for type-checking and that import Mori, Flow will load those definitions automatically. mori-spec.js is modified slightly to signal to Flow that it should type-check that file. This is to make sure that the type-checker does not infer errors in any code in the unit tests - that would indicate that there is something wrong in the type definitions. types-spec.js has been added to provide certain test cases that should pass type-checking, to make sure that those cases do pass. types-spec.js includes type annotations as specially-formatted comments. flow-bin has been added as a development dependency, and .flowconfig has been added to configure Flow type checking. Type checking can be run with: $ flow check This will check code in mori-spec.js and in types-spec.js against the definitions in mori.js.flow.
Tracks type alternation for the first five pairs. After that, only checks that arguments match either the key type or the value type. Does not prevent trying to create a map with an odd number of arguments.
On upgrading to Flow v0.22.0, the previous type definition for `partial` resulted in a type error when checking `spec/types-spec.js`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I am a big fan of Mori. I once wrote a blog post on it.
More recently I have been enjoying the benefits of type-checked Javascript. I think that Flow is especially good at describing and checking Javascript idioms. I get a lot of value from type-checking: it makes my code easier to refactor, it helps with documentation, and I get code that I trust. But when I use third-party data structures I lose type-checking, because Flow does not know anything about types in third-party code.
In this pull request I added type declarations for Mori functions in
mori.js.flow
. Flow will automatically read those declarations when Mori is included as a dependency. The type information will help users to catch silly problems early, whether they mark up their code with type annotations, or use Flow to check unadorned Javascript.Because Flow is built to describe Javascript idioms, it can express types that are not possible in some strongly-typed languages. For example, a number of the type signatures that I wrote are aware of type-broadening. Here is the signature for
concat
:The signature says that if collections containing different types of values are concatenated, the resulting sequence may contain values of types from any of the input collections. For example, if a vector of numbers and a vector of strings are concatenated, Flow will be aware that a value from the combined collection might be string or might be a number.
I was also able to capture the ability for
map
,mapcat
to zip collections. The type signatures allow for an arbitrary number of collections to be combined. But for up to five collections, Flow will be able to check that types of values in the input collections match types of argument positions in the combining function.The pull request includes some other changes:
mori-spec.js
so that Flow can check code there. I wanted to make sure that Flow did not flag any errors in the spec, since that would indicate that the type declarations are overly-restrictive.flow-bin
to the project dev dependencies, and added a script inpackage.json
. That allows for running Flow to check the specs withnpm run typecheck
.types-spec.js
to test that some slightly tricky code patterns pass type-checking.types-spec.js
does not have many assertions; but it does have type annotations in the form of specially-formatted comments.Note that the type declarations apply to the Mori v0.3 API. I could modify them for v0.5; but I don't yet fully understand how to build v0.5.
I understand it is likely that you do not want to take on maintenance burden of a big pile of type annotations. I am happy to take ownership of issues relating to type-checking, and to handle updating type declarations for new versions of Mori's API. Having type declarations in Mori will dramatically improve my experience when using Mori in my own projects.