Skip to content
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 flow types for all public APIs #2851

Closed
davidtheclark opened this issue Jul 11, 2016 · 4 comments
Closed

Add flow types for all public APIs #2851

davidtheclark opened this issue Jul 11, 2016 · 4 comments

Comments

@davidtheclark
Copy link
Contributor

I create a Flow definition file of this library for Mapbox Studio, and chatted a little with @lucaswoj about putting that definition in this repo.

I can think of some reasons that it would be a good idea to maintain the definitions in this repo:

  • Closer to the source code
  • Updates would always go out with new releases
  • If we ever figure out a way to autogenerate the definition from the thorough JSDoc comments it can happen here

I can't think of any reason not to put the definitions here. Can anybody else?

And if we do put them here, where should they go? I'm happy to open a PR.

@lucaswoj
Copy link
Contributor

The flow definitions can go in this repository's root directory. PR away!

@davidtheclark
Copy link
Contributor Author

Are all the properties exposed in https://github.com/mapbox/mapbox-gl-js/blob/master/js/mapbox-gl.js intended to be part of the public API?

If so, we should document version, util, and config, and add them all the Flow definition.

(I was thinking about this because Studio uses mapboxgl.config so needed it in the definition.)

@lucaswoj
Copy link
Contributor

  • 👍 version I added this property and failed to document it. This can and should be documented.
  • 👎 util There has been a lot of debate about whether or not we should document this. The current consensus is that we should not. Unexport or document mapboxgl.util #1408
  • config I'm not aware of any existing discussion about documenting this property. My opinion is that we should look into simplifying the architecture and then document it. A simpler architecture would be dropping the config module in favor of top level properties on mapboxgl.

This was referenced Jul 14, 2016
@lucaswoj lucaswoj changed the title Add Flow definition Add flow types for all public APIs Mar 27, 2017
@lucaswoj
Copy link
Contributor

We now have flow built in! Reframing this ticket to be about adding flow types to all public APIs.

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

No branches or pull requests

2 participants