[RFC] Base definition modules

[Brianzchen]

RFC style inspired by reactjs

Summary

This RFC aims to propose a strategy to handle cross library definition type reuse while maintaining the current cli tooling architecture. This is done through bolting on an ability to add base definition modules where library definitions rely on a list of base (primitive) modules when necessary to share type or variable definitions.

This feature should resolve issues referenced

• Allows Move type import inside of declaration #3882 to be fixed
• Support modular definitions #627 reuse across library version bumps
• Alternative solution to [WIP] Bolt monorepo for NPM migration #3483
• Alternative solution to Publish libraries as npm packages #990
• Alternative solution to Migrate to Lerna & publish libdefs to NPM #3083
• Solves Is it currently possible to have dependencies between library definitions? #1857
• Solves Can't import module types in Flow library definitions #2023
• Solve Question: shared types between packages #2842
• Provides way forward for Support modularized NPM packages #2488
• Allows stable improvements for [react-redux] Should redux's Dispatch be covariant in type #2279

Current proposed implementation

• Create base lib definitions #4116

Basic example

// --- base definition
declare module '@flow-typed/base-test' {
  declare type Test = string;
}

// --- libdef A
declare module 'libdef-a' {
  import type { Test } from '@flow-typed/base-test';

  declare module.exports: (options?: Test) => void;
}

// --- libdef B
declare module 'libdef-b' {
  import type { Test } from '@flow-typed/base-test';

  declare export default  {
    prop: Test,
  };
}

// --- consumer.js
import A from 'libdef-a';
import B from 'libdef-b';

const a = A(B.prop); // this will pass but also not be any
const a = A(123); // this will error

Motivation

In the Javascript ecosystem there are some libraries that serve as a core dependency to other libraries (think react, redux, koa, express, rxjs). These can more or less be called frameworks that are not part of the core javascript feature set yet need a level of reuse across themselves and other libraries.

Lets take a simple example of a popular frontend library redux. redux is a core library that implements an immutable store that can then support both middleware and integration libraries such as redux-thunk or react-redux respectively.

Looking at react-redux it holds a useDispatch function which returns a dispatch function which should have the same type definition as dispatch from redux. For this to be in sync, I would currently need to maintain both Dispatch and useDispatch which as you can see they are quite different. useDispatch is more complicated and has the ability to form better type inference based on whether it's passed a thunk.

This highlights one of the major problems of our current system, duplication and desync of type definitions that should have a single source of truth. Which leads library definitions that rely of core libraries with large type definitions to generally be written with any to avoid massive unmaintainable duplication and thus a less type safe and less useful tool.

Current CLI limitations

Flow itself supports cross definition imports by default no problems as longs as it's able to resolve the import. For example,

declare module 'libdef-a' {
  import typeof { mkdir, stat } from 'fs'; // is available from flow's internal definitions
  import type { Day } from 'date-fns'; // if we have date-fns installed we can resolve it from node_modules
  import type { Foo } from 'foo-module'; // if I created a module in my project
  import type { Dispatch } from 'redux'; // works as long as I also have the `redux` type definitions also installed
}

As you can see, these will all work and you can try it yourself. Looking closer at redux as a flow-typed libdef, the only blocker here for a user would be that if redux wasn't installed, we don't currently have a system that would install redux for the user if they installed libdef-a.

Further to this, redux has a few versions 1-4 (which we have 3-4 typed), which version of redux would an installer resolve to if the user installed libdef-a?

---

Next if someone were to modify the type definition for a reused type in the flow-typed registry, how do we ensure that a library definition consuming this definition doesn't break? We could run our test suite, though running every single test blindly would be very slow filled with lots of errors from old unmaintained library definitions, so we'll need some way to figure out which definitions were changed, what they impact and add them to the test runner.

The way our tests run right now is that for each definition it runs a test suite per flow version range and injects only the tests and definition file in the package/flow dir.

Detailed design

Based on the information above, the core building block of this solution are base definitions. These are definition modules that live in a new directory definitions/base which is a sibling to where current definitions coming from npmjs live (definitions/npm). Regular library definitions will then rely on these when they need reuse.

Rules of a base definition

• They must be declared as @flow-typed/base-[module]. @flow-typed to avoid name collision from real npm libraries and base- to segregate these specific kinds of definitions created by flow-typed from any other we may create in the future as all definitions under @flow-typed may not be a base definitions eg: core-[module]
• They cannot depend on any other base definition nor any npm library definition (definitions built into flow are ok, eg: fs, react)
• They will have dedicated flow version ranges just like npm library definitions
• The file will be named concatenating the module name + flow version eg: base-redux_v0.134.x-.js (this is necessary for specificity of flow versions but also a requirement of the current cli)

Once a base definition has been created a library definition must define it as a dependency in a package.json file in the flow version directory. The file will have a single property for now baseDependencies which holds an array of the names of the base definition files it will need to depend on.

{
  "baseDependencies": [
    "base-redux_v0.134.x-"
  ]
}

This allows for two things to happen

1. When a user installs a library definition they will also have the new base definition installed/updated also
2. Tests can run correctly where if the base definition is changed, we can scan the project for all package.json's and add the library definitions to the test suite if they depend on this base definition, or if a library definition has been changed we can easily check the package.json to add appropriate base definition for testing.

Base definitions do not have tests, they are tested by other definitions and any changes to a base definition would cause their tests to run. They are also considered private and there is no way to install base definitions without installing a definition that first depends on it. With this in mind, it makes base definitions very safe to tweak and overhaul without impacting the end user.

Using base definitions from a library

If I build a library that ships with flow and want to leverage base definitions. Take redux again for example, I'm building a library that uses DispatchAPI originally from base-redux, I would need to have redux as a dependency first, redux should expose this type so I can import it directly from there instead of the base definition which could change at any time causing breaking definitions without me as a maintainer doing anything.

When I publish this to npm and a user installs my library as a dependency, they will run flow-typed install because my library ships with flow it will scan its dependencies to install the redux library definition which in turn installs the base-redux definition.

Flow version breakdown

Just like library definitions, base definitions may use syntax that isn't available in older versions of flow so although they don't target package version ranges they will need to target flow version ranges.

If a library depends on a base definition it would need to be broken down to also be compatible with the flow version range it depends on. Here is an example

redux-thunk is currently typed with a range of 0.83.x-
redux is currently typed with a range of 0.83.x.-0.150.x & 0.151-x-
They now depend on base-redux which is broken down as 0.50.x-0.99.x & 0.100.x-

As the dependency is created redux-thunk is forced to be broken down to 0.83.x-0.99.x & 0.100.x-
And redux will be broken down to 0.83.x-99.x & 0.100.x-0.150.x & 0.151.x-

You can think of it this way, just as flow syntax forces you to break down your definition so too does base definitions

Drawbacks

• Because base definitions are just declared modules they can in fact be imported and used by any project once it's been installed as a sub dependency though it's not something we advise as they are private and open to changes to make them more compatible to library definitions
• All users that use a definition that imports base definitions will need to upgrade to the latest cli version, and looking at npmjs metrics we still have a substantial user base that use version 2.x.x or an older 3.x.x version (we are currently on 3.6.1)

Alternatives

Depending on other library definitions

An easier approach to this solution is instead of adding a new concept of base definitions we could just improve our testing suite to allow changes in one definition to run tests on another dependant definition.

Pros

• Simpler change

Cons

• Depending on one another could cause circular dependencies
• No place to store very low level primitive types that could be reused across many unrelated definitions
• Would not install dependant definition to a users project automatically
• One definitions say redux-thunk must depend on a specific version of redux despite having actual usage being compatible with a range of versions
• Will cause mass confusion when one definition depends on many definitions causing flow breakdown compatibility issues

Publishing to npmjs

Making each definition into an npm package and publishing them with dependencies against other definitions. This solution is targeting the movement of installation from npmjs instead of the current cli tool, not a for/against of base definitions.

Pros

• Download metrics can be owned by npmjs
• Closer to TS implementation which is familiar

Cons

• Package/flow versions can't easily follow semantic versioning
• Already published libraries won't have a dependency to a flow library thus making it harder to install dependencies vs current flow-typed install magic command
• High effort to overhaul to a new build system unless massive advantage

Not implementing anything

If we don't do anything at all do we gain anything?

Pros

• No effort for an already working stable system

Cons

• Doesn't solve cross type dependencies which makes flow-typed an incomplete system

Adoption strategy

Once the solution is merged as long as no definitions are using base definitions there will be zero impact, as soon as we migrate a single library definition we must release a new breaking v4 version and anyone using them must upgrade to continue using flow-typed.

There should be no reason to not upgrade to newer versions given all recent changes have just been non-breaking enhancements over time that have made the user experience better.

How we teach this

The documentation and mostly the contribution area needs to be updated as part of this change. We should also put something on the main readme page to alert users they must migrate to newest v4 if they are depending on a migrated library definition.

Other than knowledge for maintainers it should be very seamless for consumers who only run flow-typed install and go about their lives.

Unresolved questions

I have no opinion at all about how we should name a base definition.

For example with redux I would think calling it base-redux should be fine because between versions the API has remained the same.

But with Koa v1 and v2 are different and it would make sense to have base-koa-1 and base-koa-2 but also knowing that koa@2 is basically stable and I can't imagine a new major version coming out naming it base-koa without migrating koa@1 to use base definitions would also be fine.

Also with primitives lets say we want to have a reusable type to describe loose objects declare export type LooseObject = { [key: string]: any, ... }. What should we name this base definition? Maybe base-primitives or something.

I really have no preference but overall as mentioned above about how base definitions should be private to library definitions I don't believe we need to have any strict naming convention because if I start with base-koa and then koa@3 comes out I need to now break it down to base-koa-2 and base-koa-3 the tests will break, they'll be fixed and when a user runs flow-typed install everything will resolve as it should without anyone noticing. Unless they decided to import types directly from a base definition but I also think that's up to their own discretion just like it is if you choose to import a private module inside a library (`import blah from 'redux/lib/secretModule').

[gantoine]

They must be declared as @flow-typed/base-[module] to avoid name collision from real npm libraries

Will the existence of libraries like base-x, base-ui, etc. cause collisions? My understanding of this is that they'll be nested under the @flow-typed folder, so there won't be collisions even if base-ui get typed.

They cannot depend on any other base definition nor any npm library definition

How likely are we to encounter this kind of dependence tree: react-formic -> react-redux -> redux? And if we do encounter them, how would be address the base definition?

baseDependencies

I really like the idea of a config file for a definition; I feel like we could really build on that in future projects. 👀

Base definitions do not have tests, they are tested by other definitions

Is this a limitation of the design or something you think wouldn't be very useful? I don't see why we shouldn't allow people to write tests for base definitions, even if we don't require them.

If I build a library that ships with flow and want to leverage base definitions...

This is really cool, but I wonder how many maintainers would adopt the base definition approach outside of this project. Maybe we can reach out to some of the bigger projects written in Flow/that ship with their own defs and see what they think?

All users that use a definition that imports base definitions will need to upgrade to the latest cli version

Is there any way, even for just a couple months, that we can release this and still offer the existing definitions? Thinking out loud, I suppose the new version could reference a branch other than master where we'll start the work transitioning a libdef or two to use base definitions. That would give us time to test the idea in the wild and alert users still on V2 and V3 that they need to update...

If we don't do anything at all do we gain anything?

We should poll the discord and see how many people have been bitten by this issue and could use a solution like this. Not that it's a representative number, but if anytime we could gather some specifics and see how well this would apply.

All in a, great RFC and a very interesting solution! 👏🏼 I'll probably have more to add in a day or two.

[Brianzchen]

Will the existence of libraries like base-x, base-ui, etc. cause collisions? My understanding of this is that they'll be nested under the @flow-typed folder, so there won't be collisions even if base-ui get typed.

Yes there will never be a collision, I more meant that as long as we nest them we won't have a collision (let me go tweak and make that more clear). The base- prefix is just to allow for easier readability because so far base defs are the only custom module type we have, in case we one day support core we don't want to introduce a breaking change.

How likely are we to encounter this kind of dependence tree: react-formic -> react-redux -> redux? And if we do encounter them, how would be address the base definition?

I can't say for sure off the top of my head but if the above were the case, all the reused types across all 3 would push to eg base-redux and be they would pull from there.

Is this a limitation of the design or something you think wouldn't be very useful? I don't see why we shouldn't allow people to write tests for base definitions, even if we don't require them.

I don't think it would be useful. My thought process here is that if we're building a tailored solution, if we have a piece of functionality it should naturally encourage people to use it. As someone who's written a lot of definitions, the tests are the most tedious part and encouraging people to write tests for something that will be tested later is duplication and would make people lazy. Because of this duplication I don't see it having any additional value.

I'll use an example.

// base def
declare module 'base-a' {
  declare type Foo = string;

  declare var func: (param: Foo) => void;

  declare var unused: () => void;
}

// definition
declare module 'my-lib' {
  import typeof { func } from 'base-a';

  module.exports: {|
    myCb: func,
  |};
}

// definition test
import myLib from 'my-lib';

describe('my-lib', () => {
  it('works', () => {
    myLib.myCb('test');
    // $FlowExpectedError[incompatible-call] takes no args
    myLib.myCb();
  });
});

Because of this idea of private base definitions, my-lib completely tests everything used from base-a if I were to add tests, they would be pretty much the same, and if another lib imports base-a it should not consider whether this or that is tested and whether it needs to add tests, it should stand independently on it's own and test all of it's API because we don't know if some day this other lib would not depend on base-a and it would basically have zero tests for what it's api is where ever they may originate from.

You may also notice unused in base-a which has no tests and is also not used. This is show casing that although it isn't tested it is also unused and considered dead code. As soon as a libdef decides to consume it, it would have tests against it from the consuming lib so we're again safe there.

This is really cool, but I wonder how many maintainers would adopt the base definition approach outside of this project. Maybe we can reach out to some of the bigger projects written in Flow/that ship with their own defs and see what they think?

Very few do from my understanding as I see many shifting to TS. I more wrote this section as proving it's compatibility for future improvements and having an understanding of what may break as someone who's written and shipped a few libs with flow.

I think recoil is one but they basically have zero dependencies.

Is there any way, even for just a couple months, that we can release this and still offer the existing definitions? Thinking out loud, I suppose the new version could reference a branch other than master where we'll start the work transitioning a libdef or two to use base definitions. That would give us time to test the idea in the wild and alert users still on V2 and V3 that they need to update...

This sounds like a great idea actually! I'm sure we can point the cli to pull cache definitions from this other branch instead of default master on v4, then when we can release v4.1 at some point which targets master again.

Code ref here

We should poll the discord and see how many people have been bitten by this issue and could use a solution like this. Not that it's a representative number, but if anytime we could gather some specifics and see how well this would apply.

Can I argue that the issues list I posted at the top speaks for itself? But a poll sounds great too 😄

[Brianzchen]

I found we have this https://github.com/flow-typed/flow-typed/blob/master/cli/src/lib/libDefs.js#L407. Could be a way to alert that a libdef is not compatible with current version of cli. Haven't checked how it works yet though

[noahehall]

this would be great, im currently thinking through how I could declare export type * from 'some-library-def'

[Brianzchen]

I don't fully believe in this solution anymore and I'd like more feedback and ideas. My main concern is that as I've been typing out libs from sindresorhus in particular, a lot of the TS types reuse from his other libs.

A base def approach here could work but it would cascade into a list of base defs that all contain one type, alternatively we could have a base def which is called sindresorhus.js. Neither feels perfect but either could honestly work and it's all semantics in the end.

[Brianzchen]

Depending on other library definitions

This is fleshing out an alternative to the original proposal that may suit better given a realisation of how npm packages frequently depend on each other.

Solution

Definitions on particular flow versions that would like to depend on another will have a package.json at the flow dir level.

foo_v1.x.x
  |- flow_v0.83.x-
      |- package.json
      |- foo_v1.x.x.js
      |- test_foo_v1.x.x.js
      |- ...
  |- flow_v...

The contents of this package.json will hold a list of deps similar to a normal package except that the value of each property will be an array of strings holding the version.

// react-redux_v8.x.x/flow_v0.83.x-/package.json
{
  "deps": {
    "redux": ["3.1.x", "v3.x.x", "4.x.x"]
  }
}

This allowing for solving the issue of a single libdef being compatible with many versions of of a dependency whether major or minor.

---

With this dependency established, when a consumer installs react-redux@8 flow-typed will check whether they have a dependency to redux of one of the valid versions we support, if so we do no extra work, flow-typed will already install that version for us.

If they do not have a dependency to redux nor will it be installed by a nested flowtype dependency then we need to install it for react-redux types to function, we will default to the last version in the array which we expect to be the latest version though that's up to the definitions maintainer to decide.

Can recursively install dependencies

---

From the repo side to support this change:

• If a change is made to a definition that is depended on by another definition, the tests of the definition and the dependants should be run
• Because the test runner runs all flow versions across a range at once, for multiple dependencies to run in sync all definitions need to be aligned on flow versions of the versions that contain a dependency
• If a change is made to a definition flow version with an array of dependencies, it needs to recursively run the definition tests against each version of a dependency
• If a change is made to a definition flow version with multiple dependencies it should install one version from all to test against
• Can recursively test dependencies

[Brianzchen]

Adoption strategy

The major risk of a change to the way definitions are consumed is that are used by many people and we only hold a single version at a time.

Open to ideas of how we manage this, whether we pull definitions from a feature branch first while the code sits in master or separate dirs for package.json supported directories