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

Which interfaces are already written? #13

Closed
chrisbolin opened this issue Mar 10, 2016 · 7 comments
Closed

Which interfaces are already written? #13

chrisbolin opened this issue Mar 10, 2016 · 7 comments

Comments

@chrisbolin
Copy link

It might be nice to get a list of interfaces to "seed" this project with.

As @marudor mentioned, he has collected a number of interfaces here:
marudor/flowInterfaces
I can vouch for a few of these.

I also found TechnologyAdvice's interface repo (which has overlap with marauder's) and a jQuery interface.

I'm pumped we're finally getting these together and am happy to help any way I can! 🎉

@mrmurphy
Copy link
Contributor

Good idea, @chrisbolin. Thanks for posting those!

I'll link another one in here, the immutable.js definitions that are in progress:
immutable-js/immutable-js#203

Specifically, this one by @halletj
immutable-js/immutable-js#203

@danez
Copy link
Contributor

danez commented Mar 12, 2016

I think it would be also a good idea too move/copy the react definitions from the flow repository itself to here. Although a lot of people probably rely on them currently, so there should be some strategy on how to clever move them without breaking flow for a large group of people. Maybe a deprecation warning if one uses the react definitions from flow itself and how a short message to migrate to this repository.

@jeffmo
Copy link
Contributor

jeffmo commented Apr 20, 2016

FWIW: I think it would be great to start pulling these libdefs in now.

Most of the remaining work on flow-typed is related to building out the CLI to make it easier to acquire the libdefs -- but seeding the repo (and having a nicely central place for people to come and find them manually) would still be very useful.

@mrmurphy
Copy link
Contributor

+1, especially @marudor if you have time.

@marudor
Copy link
Collaborator

marudor commented Apr 23, 2016

As you can see by the PRs I've put my libdefs in here. Now it's time to review them.

@jeffmo
Copy link
Contributor

jeffmo commented Apr 26, 2016

Thanks @marudor!

This set of PRs has been really helpful in fleshing out the process of review. Here are some thoughts so far:

Reviewing Test Files

  • Every definition file should have at least one test_ file that exercises it.
  • Each def file should be tested in at least the following ways
    1. Import all declared modules (make sure the act of merely using a libdef doesn't cause flow errors)
    2. Verify that all globals expected to be declared are visible in the test file
    3. Include both positive and negative tests to ensure that type checking is actually happening (AKA: Use // $ExpectError in at least a few places to ensure that we're getting type errors in a few critical places we would expect them)
    4. Include type assertions about at least a few items declared by the definition file (i.e. positive test: (new Widget().install(): void); and negative test: // $ExpectError \n (new Widget().install(): number)

Reviewing Library Definitions

  • I've been looking closely at the docs for the library being defined and comparing that with the libdef
  • In general definitions should avoid using any types (which also includes Function and Object) unless strictly necessary. In many places, using mixed in place of any in definition files will work and is a bit safer because it means that if the type ever leaks out of the definition somehow, it will need to be explicitly refined by the user before it can be used. I've opened [Test CI] Automatically comment on PRs that add any types #49 to track adding a feature to validate-defs that comments on a PR when it introduces an any type explaining that this should be avoided if at all possible. We don't want to ban any types completely (sometimes they are strictly necessary), only discourage their use if there is an alternative.

I think that's all the patterns/thoughts I've noticed so far, so let's add more here as things progress and eventually put them into the readme!

@ryyppy
Copy link
Contributor

ryyppy commented Jun 13, 2016

I close this issue, since there is an established process and a list of definitions now :-)

@ryyppy ryyppy closed this as completed Jun 13, 2016
cullophid pushed a commit to cullophid/flow-typed that referenced this issue Jun 19, 2017
Make it so that !important can be disabled for certain style rules.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants