Dependencies between library definitions?

[samwgoldman]

I started writing this out in the facebook/flow repo, but I think it actually might belong here. At least I think the discussion is more appropriate here, and might lead to tasks for Flow.

In a world where flow-typed manages library definitions for things like React, how should people write definitions for their libraries that depend on React types? Currently we allow library definitions to export global types, but I'm only interested in modular proposals.

I think it's worth separating libraries, like React, from environments, like node or browsers. It's possible that the same thing might solve both cases, but maybe not.

Library definitions currently describe compatibility with version(s) of the library itself and version(s) of Flow, but what about dependencies to other library definitions? What about node libraries vs. dom libraries?

[samwgoldman]

Ref facebook/flow#1543

[marudor]

I think flow needs a setting for the env (node/browser)
Based on that flow itself will provide basic libdefs for the globals.
(Like Number, Promise, Math and so on)
For the node env also fs, path and everything that comes with node.

Everything else should be modular. So the current react defs in flow should be in here.
We would also need to be able to use import in defs. Currently it only works partially as the referenced Issue shows.
In my time writing libdefs I often noticed I need to import stuff. So far I've worked around it by declaring a global type prefixed with the library name just like the flow defs. But this will probably run into issues and collisions sooner or later.
It will probably make it easier and faster to update definitions as well. My PR about updated libdefs in the flow repo is still open (since mid December).
Hopefully by seperating libdefs and flow code it will be easier to keep updated.

[jeffmo]

On libdef dependencies:

I hadn't really thought about this, but it makes sense. So to just start by circling the simplest solution: How about a dependencies.json metadata file?

dependencies.json

{
  "react": "~14.0.0"
}

On importing things within a declare module:

I've been wanting to tackle this for a little while now -- I've just been waiting on someone to really need it! What I had in mind was to take declare module {} and add support for toplevel export statements within it (i.e. export function foo(bar: BarType): RetType;). TypeScript came to the same conclusion already and I think it makes a lot of sense.

What follows from this is that we'd also have room for toplevel import statements inside declare module {}. I would like to avoid supporting import statements outside of declare module{} in libdef files due to it's semantic ambiguity ("all other variables at toplevel become globals, shouldn't imports also?"). If we find cases where it's strictly necessary I'm of course open to exploring it (or possibly some alternative libdef syntax)...but I would much prefer to only tackle this if we must.

On platform libdefs (dom/node/react native/etc):

I think these should ship with Flow and be toggle-able. I generally think all platform libdefis should be on by default (everything works out of the box) and opt-in for specificity. My reasoning here is two-fold:

1. Working out-of-the-box with minimal decision making is really important.
2. There are relatively few distinct platforms, so collisions are rare/tractable for a "unspecified platform" Flow default configuration.
3. If Flow understands more APIs out of the box than what you use then that is strictly better than it not understanding an API that you try to use (requiring you to figure out why and how to enable it). Moreover I believe it is relatively rare to begin working on a project and accidentally attempt to use an API from an unintended platform.

For example:

.flowconfig that includes core.js + node-<<latest>> + dom.js + bom.js (...etc...) from built-ins:

[options]

.flowconfig that includes only core.js + node-v5.js from built-ins:

[options]
platform=node-v5

.flowconfig that includes only core.js + dom.js + bom.js from built-ins:

[options]
platform=browser

The main counter to (3) that I have heard is that some might consider a default of understanding APIs that don't exist on the intended platform to be an out-of-the-box hazard (i.e. Flow wouldn't give an error by default when attempting to use an API that doesn't exist on the intended platform).

What do others think here?

[marudor]

I'm more a fan of the opt-in version instead of opt-out.
So no config means just very basic libdefs. As in the ones from core.
It suits flows general pessimistic style.

[ryyppy]

I also like the opt-in version better.
When I think about other tools like eslint, they also provide an env config to enable es6, node, etc.

{
  "env": {
    "es6": true
  }
}

Especially in the case of react-native, wouldn't it defeat the purpose if node-env is also already switched on by default?

For the importing part:
I would not mind if an import statement would only be possible in declare module's scope. In general, I find it very confusing how flow treats module declarations and how well it isolates them from other libdefs... Apparently @marudor used the [module]$ namespace approach, but this will also collide with version-management sooner or later if I am not mistaken?

What are the next steps for this issue? Is there anything already in the pipeline?

[jeffmo]

Seems we've already encountered a need for this: #44

[Macil]

I've made type definitions for the Kefir, and then I've made type definitions for other libraries that return values of type Kefir.Stream. The problem is that having import Kefir from 'kefir' at the top of those other definitions causes Flow to think that Kefir is a global variable, which then ends up causing an exception at runtime when we miss that.

Being able to use an import statement inside of declare module Foo { ... } and declare class Foo { ... } which doesn't expose imported module as a global variable would solve most of the problem, but there's still some issues with it:

• I can't use declare class Foo extends ImportedValue { ... } without ImportedValue being treated as a global, unless imports inside of the block count as in scope.
• I can't use declare function foo(): ImportedType; without ImportedType being treated as a global.

[ctrlplusb]

Hi all, has there been any movement on this issue? I'm happy to contribute in whatever way I can.

I'm new to flow, and whilst I find it awesome, I have been taken aback by the lack of type definitions. I immediately forked this repo in the hope of contributing a react-router definition, but then came up against it as react-router depends on history. At the moment flow-typed doesn't support definition dependencies so I would be forced to create my own definitions in my project should I not want to embed the history definition within my react-router definition. I can only assume this is what lots of people are resorting to.

It would be great to get this issue moving along as the more typed we make the world will make flow that much more appealing. :)

[ryyppy]

Actually, AFAIK @jeffmo is pushing the feature for importing declarations in other declare module closures forward... For now, we are using an arbritary prefix system to namespace our libdefs, which IMO works very well and will be an ok-solution as soon as the new syntax features are settled... sadly we do not ensure if certain libdefs are installed, if another libdef depends on it...

so if you e.g use $npm$History$SomeType , flow will eventually tell that it doesn't know this type... so people should know that they need to install those aswell... I guess our testing facility will also cause problems since it tests the libdefs in isolation....

[jeffmo]

Indeed, this is on my near-term plate.

I'm currently preoccupied with the 0.28 release (which is pretty complicated as it contains fixes for unions, but at the expense of some trade-offs that we're still working through). As a result my bandwidth is pretty low right now, but hopefully in the next couple of weeks I can hop back on the horse and keep driving here.

[ctrlplusb]

Thanks @ryyppy - the namespacing suggestion is working out well for me thusfar. I'll keep on developing my definitions locally and then as soon as this issue progress I will PR them over.

@jeffmo - great to hear that it's on your plate. Good luck with the 0.28 release. :)

[karelbilek]

Hello. I am not sure if I understand this.

I want to write definitions from module bitcoinjs-lib, but for that, I need to use definitions from bigi ("just another" BigInteger implementation, that has some specific features) that bitcoinjs-lib functions use as inputs.

Is there a way do that correctly? Should I copy-paste bigi module definition to bitcoinjs-lib module definition? That seems to work, but it seems icky.

[ctrlplusb]

@jeffmo sorry, no pressure here, but just wanting to know if there was any update on this issue?

[jamiebuilds]

To respond on Jeff's behalf, he said he's planning on coming back to flow-typed to bring it to feature completeness soon. We are discussing workflows around flow-typed and .js.flow files now. I'll try to get us to publish some planning documents soon.

[CrabDude]

@jeffmo Have you started work on this yet? I have some time I can contribute, and we could use this feature.

[jeffmo]

@CrabDude: I haven't started yet so I'd love to just let you run with it. Before that we should probably nail down a design and consider its various touch-points, though.

Reviewing this thread, it seems like my suggestion to have a dependencies.json metadata file is the only standing idea -- but I would point out atleast one downside: It would be mostly redundant with the actual import statements themselves. I worry a bit about dependencies/imports rotting/going out of sync.

An alternative proposal might be to have a special declare import syntax that is only valid within declare module { ... } bodies, except it has a version clause... Maybe something like:

declare module "MyModule" {
  declare import _ from "lodash" @ "4.15.x";
}

Any thoughts on this alternative idea?

[zertosh]

I'm needing this right now. Also, support for non-npm packages.

We have types for Atom and Electron in Nuclide that we want to use in other packages. There are several issues with this:

• Atom is not an npm package. However, when running in an Atom environment, you're able to import {...} from 'atom' for something things, AND, there's also an atom global variable that has other things.
  • So, we need to distribute types for the atom pseudo-package, and for the atom global variable.
• Electron has the same issues as Atom. When running in an Electron environment, you can import an electron module, and there are other global variables available like WebView. There are also modifications to standard DOM types (File object, window.open function)
• The Atom types also need to import types from Electron. Certain Atom functions return Electron types (or take Electron types).

So the highlights are:

1. How to publish non-npm packages?
2. How to express relationships between them?
3. How to share types for environment globals?

[vovacodes]

@calebmer

You should be able to import type from within declare module. Please let me know if that doesn’t work or if there is anything else flow-typed needs to handle dependencies

I believe what makes sense to have is an ability to specify the version of the dependency you are importing types from

[qm3ster]

Is there a tracking issue for platform/env config? Separating dom.js from WebWorkers would be useful.

[Dr-Nikson]

@calebmer hello!
I'm trying to create typings for @most/create. It has TS typings, but the problem is - these typings based on type Stream that needs to be imported from most. And most doesn't have typings in flow-typed repo (they are placed inside node_modules/most/lib/index.js.flow) - as the result I'm getting module not found error

flow-typed/npm/@most/create_vx.x.x.js:

// flow-typed signature: 319db8c4ec38ff5487d595965e709ea7
// flow-typed version: <<STUB>>/@most/create_v^2.0.1/flow_v0.63.1

declare module '@most/create' {
  import type { Stream } from 'most'; // not found

  declare function create<A>(
    f: (
      add: (a: A) => void,
      end: (x?: A) => void,
      error: (e: Error) => void
    ) => void
  ): Stream<A>;
}

Is there any way to solve it?

[saadq]

@calebmer

You should be able to import type from within declare module. Please let me know if that doesn’t work or if there is anything else flow-typed needs to handle dependencies

I'm actually having an issue trying to do this. I'm basically trying to use a type from koa within the koa-router libdef.

Here is what I have:

/**
 * @flow
 */

declare module "koa-router" {
  import type { Middleware } from "koa";

  // ...Use `Middleware` in definitions...
}

When I try to run my libdef test (currently only running for v0.64), here is the output I get:

Fetching all Flow binaries...
Finished fetching Flow binaries.

Testing koa-router_v7.2.x/flow_v0.64.x- (flow-v0.64.0)...
 
ERROR: koa-router_v7.2.x/flow_v0.64.x-
 * koa-router_v7.2.x/flow_v0.64.x- (flow-v0.64.0): Unexpected Flow errors(2):
   Library type error:
   koa-router_v7.2.x.js:12
    12:   import type { Middleware } from 'koa';
                                          ^^^^^ koa. Required module not found
   
   
   Found 1 error
   
   null

[loyd]

Any progress?

[rostero1]

I'm kind of confused by this ticket. Are the current flow-type definitions that import types from other installed libraries not actually importing/applying them?

[saadq]

Any imports in the current libs are getting assigned an any type. For example, in body-parser:

import type { Middleware, $Request, $Response } from "express";

In that line, Middleware, $Request, and $Response are equivalent to any.

[jedwards1211]

@samwgoldman I don't know why the discussion started out primarily about React, that's only the tip of the iceberg. This is a serious, wide-ranging issue. For instance, it's currently impossible to guarantee that a bonafide graphql DocumentNode is passed as the query property of a react-apollo Query component.

[tim-stasse]

This is certainly a major issue with being able to successfully use flow. Currently it's possible to import and reuse types from other libdefs (extending an imported class is a seperate issue though..).

Importing types from a library with existing types however, is impossible, and instead flow reports the module as not found.

The workaround for myself at the moment is to duplicate the types of the library in a libdef, which allows the types to be imported in other libdefs. The issue I have atm with this, is that as @jedwards1211 points out, the types detected by flow conflict and are not compatible. For some reason specifically for the DocumentNode type in the graphql library. This has then forced me to add an ignore for the graphql node_modules folder in my flowconfig to make it work.

All of this is just work around though, and feels wrong. It also certainly not ideal to have to duplicate types that should just work.

[AndrewSouthpaw]

We would love for this conversation to be actively discussed in the Flow repo, since we actually have nothing to do with the problem. It is a serious problem, but we have no control over it.

[Dr-Nikson]

@AndrewSouthpaw can you provide a link to this issue in the Flow repo, please? I haven't found something similar there.

[AndrewSouthpaw]

I'm unaware of one. If one doesn't exist, start it. :)

[tim-stasse]

I've commented on this one here: facebook/flow#3883