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

Migrate to Lerna & publish libdefs to NPM #3083

Open
1 of 6 tasks
villesau opened this issue Jan 16, 2019 · 53 comments
Open
1 of 6 tasks

Migrate to Lerna & publish libdefs to NPM #3083

villesau opened this issue Jan 16, 2019 · 53 comments
Labels
release Related to the next major release

Comments

@villesau
Copy link
Member

villesau commented Jan 16, 2019

What we gain with this?

  • Lerna setup allows us to publish library definitions to NPM.
  • Using NPM allows to utilize npm dependencies for libdef dependencies.
  • You can live without flow-typed CLI if you want to

What it requires?

Open questions:

  • How the libdefs should be named? DefinitelyTyped uses "@types/uuid": "^3.4.2", way of naming. The problem with that is that Flow moves fast and we need to support multiple Flow versions for different libdefs. How to achieve that?

    • Perhaps the naming could be something like: "@flowtyped/uuid-flow_v0.32.x-": "^3.4.2" ?
    • Support only latest Flow version
      • Is this feasible? Most likely not since so many people lack multiple versions behind.
  • Should CLI & test runner be split into own repo?

Some earlier plans: https://docs.google.com/document/d/1bR1k70q5xZfajlbbCJR_ZYC-UAU-aH1qUY54RxafNGY/edit

Some experimenting: master...villesau:run-tests-against-experimental-folder

@villesau villesau added the release Related to the next major release label Jan 16, 2019
@doberkofler
Copy link
Contributor

I would not mind to only have support for the latest (or maybe last 2) Flow versions

@villesau
Copy link
Member Author

villesau commented Jan 16, 2019

@doberkofler Thanks for the input! I think the choice is between latest version (with a possibility that the libdef might be backwards compatible to certain extent) or almost any version (currently we test against 15 latest versions). How DefinitelyTyped does it is that they have the actual library version (^3.4.2 in this case: "@types/uuid": "^3.4.2", see https://www.npmjs.com/package/@types/uuid) as a version, and they just override the version in NPM with updated libdefs. This means that when ever libdefs is updated with breaking change, it wouldn't work with older Flow versions -> you need to update Flow as well.

@doberkofler
Copy link
Contributor

@villesau I would go for the latest version to keep it as simple as possible. I have been using flow for quite some time and like it a lot but am currently thinking to migrate to TypeScript specifically because the availability and quality of the type definition files for TypeScript.

@jamesisaac
Copy link

I'm not sure you need Lerna for this. It would add unnecessary boilerplate to each typedef folder, and probably not something Lerna is designed for anyway (dozens of packages, yes; 100s/1000s, no).

How about keeping the folder structure like it is now, with a script that programmatically adapts each folder to the relevant package(s) and publishes to npm. Afaik this is how TypeScript does it.

@zeorin
Copy link

zeorin commented Jan 16, 2019

What about using tags to mark which flow version ranges a libdef version is compatible with? That way people that need to remain on an older version of flow can get the latest version of the libdef that's still compatible with their flow version.

@villesau
Copy link
Member Author

@jamesisaac Then we would need custom tooling for that. I believe Lerna gives most of that for free. The format of packages would follow identical structure so in that sense it should not matter wether there is 2 or 2k packages. But the approach still needs to be validated. We've heard that Yelp uses this kind of method for their internal libdefs.

It would add unnecessary boilerplate to each typedef folder

What boilerplate it would add besides package.json? package.json would be pretty neat way to manage dependencies in standardized way.

What about using tags to mark which flow version ranges a libdef version is compatible with? That way people that need to remain on an older version of flow can get the latest version of the libdef that's still compatible with their flow version.

@zeorin This is an interesting idea! I've never used tagging so we might need to investigate this option a bit. How would I specify version in package.json with a tag? How would "@flowtyped/uuid": "^3.4.2" look with tag in it?

@jamesisaac
Copy link

Maybe you're right there. I'm not quite familiar enough with Lerna or flow-typed to envision exactly how a Lerna-based structure would work. But I'm just thinking, especially if you're wanting to do something fairly custom with tagging package versions based on a few variables, Lerna probably won't support that out of the box. So the options would either be (a) complex layout of typedef folders with some duplication to make Lerna happy, (b) wait for Lerna to build in upstream support for a specialised use case, or (c) custom tooling.

@jamesisaac
Copy link

Another thought on solving the multiple Flow version support:

What about if typedef folders only contain code targeting the latest version of Flow, and some kind of codemod is used to make extra transpilations of it which support older versions at publish time, with a line in .flowconfig [includes] which points to the correct output for the user's Flow version.

The task of turning newer Flow constructs into the best approximations possible with the earlier versions would be a significant upfront investment, but the alternative of having packages like "@flowtyped/uuid-flow_v0.32.x-": "^3.4.2, each based on independent sources, does sound like a bit of a nightmare.

Not sure if that approach is actually feasible in practice, but if it worked it would scale to all typedefs, so save huge amount of work on maintaining backwards compatibility. Bug fixes and improvements to typedefs would also be able to reach older Flow versions.

@jbrown215
Copy link
Contributor

How the libdefs should be named?

I'm very interested in seeing a solution to this problem. At some point, I'd like to be able to version Flow's react.js libdef and I will probably recommend that we follow whatever scheme you come up with here.

@villesau
Copy link
Member Author

villesau commented Jan 17, 2019

What about if typedef folders only contain code targeting the latest version of Flow, and some kind of codemod is used to make extra transpilations of it which support older versions at publish time, with a line in .flowconfig [includes] which points to the correct output for the user's Flow version.

I'm afraid that it wouldn't work in practice though. Flow often has breaking changes thats not possible to fix using codemod. And sometimes libdefs require major refactoring or even rewrite. One recent example of such is react-redux.

alternative of having packages like "@flowtyped/uuid-flow_v0.32.x-": "^3.4.2, each based on independent sources, does sound like a bit of a nightmare.

Yeah this might not be the most convenient way of solving the issue.

So far tagging is the most interesting approach. What about something like this:

Latest: "@flowtyped/uuid": "^3.4.2"
With version range: "@flowtyped/uuid": "^3.4.2-flow_v0.32.x-"

Then you could install it like yarn install @flowtyped/uuid@flow_v0.32.x-" or yarn install @flowtyped/uuid@^3.4.2-flow_v0.32.x-". The version range (flow_v0.32.x-) might became problematic due to this disclaimer "Note: Since dist-tags share a namespace with semantic versions, avoid dist-tags that conflict with existing version numbers. We recommend avoiding dist-tags that start with a number or the letter "v"." https://docs.npmjs.com/adding-dist-tags-to-packages

@jamesisaac
Copy link

I'm afraid that it wouldn't work in practice though. Flow often has breaking changes thats not possible to fix using codemod.

Do you have an example of a Flow change that's particularly tricky to codemod? Quite tempted to play around and see what's possible.

And how many prior Flow versions are you be realistically aiming to support? There's code in the react-redux libdef supporting back to v0.30, which is a 2.5 year old Flow release.

And sometimes libdefs require major refactoring or even rewrite. One recent example of such is react-redux.

Well it could always allow manual multi-version support too for the 1% of libdefs that receive as much community attention as react-redux. But for the vast majority, I imagine they're just using pretty standard features, so making them backwards compatible would consist of tasks like stripping out new syntax (..., +). Quite predictable work yet removes the burden of manually having to maintain these multiple versions.

@zeorin
Copy link

zeorin commented Jan 17, 2019

Starting tag names with flow_ should prevent any collisions with semantic version numbers. It could even be shortened to starting tag names with just f.

Some examples:

@flowtyped/uuid@flow_v0.32.x-
@flowtyped/uuid@f0.32.x-
@flowtyped/uuid@f0.x

Note in the last example I used semantic versioning instead of the open-ended range notation. It's well understood and more expressive.

@PinkaminaDianePie
Copy link
Contributor

can we somehow use peerDependencies for that? like each definition package will have peerDependency with specific flow-bin versions range?

@zeorin
Copy link

zeorin commented Jan 25, 2019

That would mostly be useful to tell a user what version of flow is suitable for the current libdef they have installed (and we should certainly do that if we can, because npm and yarn print a warning when there's a peer dependency version mismatch), but it would make it hard to select a specific version of a libdef based on the current flow version.

Using tags allows us to select a specific libdef version, because they're essentially aliases for versions.

@goodmind
Copy link
Contributor

Is this same org? https://www.npmjs.com/org/flow-typed

@villesau
Copy link
Member Author

@goodmind yes.

@flowtyped/uuid@flow_v0.32.x-
@flowtyped/uuid@f0.32.x-
@flowtyped/uuid@f0.x

@zeorin what about the actual uuid version? That's not represented in these examples. uuid 1 and 2 might have very different interfaces.

@Mouvedia
Copy link
Contributor

Mouvedia commented Jan 31, 2019

Personally the only solutions that I am seeing which would provide adequate UX is having flow-typed CLI resolving intelligently any requests and giving suggestions if something is not found:

flow-typed install foo@latest@1.2.3

You requested foo@latest@1.2.3, this specific version of flow couldn't be found. Did you mean

# a maximum of 3 proposals if it fails and find other versions
- foo@latest@1.2.55 # the last patch in the same minor
- foo@latest@1.3.x # the next minor (generic or specific)
- foo@latest@1.2.1 # the last version that precede the version requested
flow-typed install foo

You requested foo@latest@latest, foo@3.14.5@2.4.x is being installed.

This way you don't have to publish the packages several times for each version of flow supported.

@villesau
Copy link
Member Author

villesau commented Feb 5, 2019

@retyui you can have more than one folder for libdefs. If there is a buggy libdef, you can pick it from node_modules and place it to your own folder. And ofc ultimately submit the fix back to flow-typed :)

@villesau
Copy link
Member Author

I played around with this idea today and couldn't find a way to manage NPM tags in scale using Lerna so ended up making an issue of it: lerna/lerna#1936

I'm happy to be proven wrong though!

@villesau
Copy link
Member Author

Looks like NPM has builtin support for default tags: https://docs.npmjs.com/files/package.json.html#publishconfig

@villesau
Copy link
Member Author

villesau commented Mar 12, 2019

Hit next problem with Lerna: lerna/lerna#1980

Also Yarn workspaces prevents from having multiple packages with same name. Not sure if yarn workspaces are needed though.

@retyui
Copy link
Contributor

retyui commented May 5, 2019

what new?

@villesau
Copy link
Member Author

villesau commented May 5, 2019

Nothing new unfortunately :/ Haven't had really time to put effort on this. Looks like Lerna (the project) is pretty difficult to setup locally. I should try how it turns out with forked @lerna/package-graph that does not have the constrains we are hitting, more info here: lerna/lerna#1980 .

It might also be worth to evaluate Rush.

@retyui
Copy link
Contributor

retyui commented May 6, 2019

@villesau
How will you version definitions?

{
  "name": "@flowtyped/axios",
  "version": "0.18.0",
  "dependencies": {},
  "publishConfig": {
    "tag": "flow_v0.25.x-v0.74.x"
  }
}

Now package.version is an axios version. And to publish a new version to npm you need update patch number "version": "0.18.1".

How about breaking changes? (if I change some names of the imported types) so... You will need to increment the major "version": "1.18.1"

Based on this, I think you understand the problem?

The package version of the package should not depend on the version of the definition. Need to revise this point

@villesau
Copy link
Member Author

villesau commented May 6, 2019

@retyui yeah something like that!

It's possible to push new versions with same version number & tag to NPM. That's how Definitely-typed does it too.

@retyui
Copy link
Contributor

retyui commented May 10, 2019

@villesau
How about continue to work based on the experience of babel lerna/lerna#1980 (comment)

@goodmind
Copy link
Contributor

goodmind commented May 21, 2019

I would just stick to this scheme
major.minor - library version
patch - flow-typed version

It's what DT does, but this isn't really SemVer

@villesau
Copy link
Member Author

Some updates:

@goodmind has done excellent research on the topic and looks like Flow gets mad if there is package.json within libdef files: facebook/flow#7746 <- this is quite a big setback. It does not crash the Flow server, but causes errors.
Also currently looks like https://github.com/boltpkg/bolt could be the best suitable monorepo tool for us.

@goodmind
Copy link
Contributor

facebook/flow#7746 was fixed, so we need to wait for next Flow version

@goodmind
Copy link
Contributor

0.99.1 was published

@goodmind
Copy link
Contributor

Sadly without fixes from PR 😒

@jedwards1211
Copy link
Contributor

jedwards1211 commented May 31, 2019

I see retyui mentioned breaking changes to type defs but i don't think people here are taking that problem seriously enough.

Case in point: Flow made breaking changes to the React type defs at one point. It wasn't motivated by a breaking change to React itself; they just decided to use a different set of class type parameters and so forth. I have sometimes thought of doing the same with the sequelize type defs I wrote.

This will happen in complex libdefs and you'll want to have a way to bump the major version of a type def package when it happens.

You wouldn't want people to wind up with incompatible type defs after they yarn upgrade would you? I think DT made the wrong decision here. The whole dream of semver is being able to upgrade with confidence.

Probably best to put a table of compatible type def versions in this repo and still rely on a CLI tool to look up the correct version of the type defs.

There might also be a way to publish version tables in the type packages' own metadata on npm, if npm makes custom parameters in package.json visible.

Having the type def version not visually match the package version might cause some pain for developers, but I think a good CLI could mostly mitigate it, and it would be less pain than unexpected breaking changes to the types.

@goodmind
Copy link
Contributor

goodmind commented May 31, 2019

@jedwards1211 just track library version as peerDependency and allow typedef to have any version that it wants

@goodmind
Copy link
Contributor

I don't think CLI is the option, because the second reason besides dependencies is to use native npm install

@jedwards1211
Copy link
Contributor

@goodmind you have no plan for dealing with breaking changes to the types though

@jedwards1211
Copy link
Contributor

would npm actually pick the highest version of a types package with a compatible peerDependency for the actual package? I'm not sure it would. I think it would just pick the highest version and potentially give you a peer dependency warning.

@goodmind
Copy link
Contributor

potentially give you a peer dependency warning

Isn't this enough?

@jedwards1211
Copy link
Contributor

@goodmind I guess, but what's wrong with optionally being able to use a CLI to automatically pick the right version?

@jedwards1211
Copy link
Contributor

As DT issues like DefinitelyTyped/DefinitelyTyped#7719 illustrate, relying solely on npm install is really inconvenient if you need to install defs for an older version of TS. We still have the opportunity to make something better for Flow. People could still use npm install if they want, but on top of that we could also offer a CLI to make people's lives easier.

@goodmind
Copy link
Contributor

goodmind commented May 31, 2019

@jedwards1211 ah, old versions wouldn't be supported because of bug with .json files in [libs]
Flow parses them as javascript and can't typecheck

I think old Flow versions would still use CLI, but it would remove .json files from node_modules/@flowtyped

@jedwards1211
Copy link
Contributor

jedwards1211 commented May 31, 2019

Yeah, but we can support all versions released after that bug gets fixed though. At some point in the future there will be "old" versions that don't have that bug

@goodmind
Copy link
Contributor

goodmind commented May 31, 2019

You mean that users wouldn't know what version they need to install? Because it would be different from package version. Doesn't tags with Flow versions solve this problem?

@jedwards1211
Copy link
Contributor

jedwards1211 commented May 31, 2019

The problem is basically to map a three dimensional space (flow version x package version x typedef revision) into a one-dimensional space (typedef package version). Computers are good at handling mappings like that, humans not so much.

To make easy for users to know which version to install you basically have to sacrifice two of those dimensions.

@jedwards1211
Copy link
Contributor

And keep in mind, people with large-scale codebases are stuck on old versions. My company can't commit time to upgrade from Flow 0.84 right now because it would take days to fix all the new errors.

@goodmind
Copy link
Contributor

goodmind commented Jun 1, 2019

You can still use CLI

@goodmind
Copy link
Contributor

goodmind commented Jun 1, 2019

Eventually I think flow-typed can migrate to .js.flow with @flowtyped/foo packages taking precedence over foo, and @flowtyped/foo__bar packages resolving to @foo/bar

@goodmind
Copy link
Contributor

goodmind commented Jun 8, 2019

Progress so far

  • Monorepo format
  • Publish libdefs to NPM
  • NPM namespace
  • Test runner
    • Half-working
  • Dependencies
    • Works with [libs] if there's no conflicting packages
    • What to do with version conflicts?

@goodmind
Copy link
Contributor

0.101 landed with .json files fixes

@Brianzchen
Copy link
Member

Any update on this one?

@AndrewSouthpaw
Copy link
Contributor

Not to my knowledge. @goodmind was working on it for a chunk of time, but then stalled out.

@FezVrasta
Copy link
Contributor

Could Flow be enhanced to support a dedicated package.json property like this?

  "flow": {
    "v0.32.x": "types/my-lib-v0.32.x/",
    "v0.106.x": "types/my-lib-v0.106.x/",
  }

So a single @flow-typed/uid package could ship multiple versions of a type def?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Related to the next major release
Projects
None yet
Development

No branches or pull requests