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

Time to deprecate gen-flow-files? #5871

Closed
ryami333 opened this issue Feb 26, 2018 · 23 comments
Closed

Time to deprecate gen-flow-files? #5871

ryami333 opened this issue Feb 26, 2018 · 23 comments

Comments

@ryami333
Copy link
Contributor

ryami333 commented Feb 26, 2018

The command gen-flow-files has been around a while now, and it's been almost untouched since it was released as an experimental command in v0.32.0 about a year and a half ago. I'm unaware of any libraries using this in production (probably because for most use-cases it just doesn't work).

I think there are dangers of leaving such a buggy/experimental command in there with no active intent on fixing/updating it:

  • Library maintainers could be waiting indefinitely for an update rather than seeking out alternative means (like flow typed or flow-copy-source)
  • Library maintainers might be publishing buggy lib-defs without realising it.
  • Or worse, they can be frustrated by gen-flow-files so much that they abandon Flow types altogether.

I think the command should be deprecated and ultimately removed, and documentation updated to reflect the official recommended pathway to publishing types, whatever that may be.

@ahem
Copy link

ahem commented Feb 26, 2018

I consider it a pretty useful tool and I am using it for a bunch of internal modules. It doesn't generate the most readable output, but I also haven't run into any problems with it.

@ghost
Copy link

ghost commented Mar 4, 2018

I'd personally like to hear the status and long-term plan on this before voting to remove it as the tool has the potential to be very useful. flow-copy-source is at best a patch for what gen-flow-files has the potential to be.

@ryami333
Copy link
Contributor Author

ryami333 commented Mar 6, 2018

Yeah I agree that it has the potential to be very useful, but at the moment it's essentially just vapourware, or at least it carries all the associated risks of vapourware.

@chixor
Copy link

chixor commented Mar 8, 2018

I disagree to calling it vapourware because we actively useflow gen-flow-files with satisfying results.

Use case
We have 50+ private flow-typed repositories.

Why we don't use flow-typed
We can't use a public repo for the flow types of private repos. We also can't use one private repo for flow types because among the 50+ repos it would have to represent there are conflicts between repos that wouldn't coexist anywhere else. For this approach to work we'd have to manage multiple private flow type repos, which adds needless overhead for us. We much prefer a single source of truth approach.

Why we don't use flow-copy-source
flow-copy-source merely duplicates the entire source code into the /lib dir. This means doubling the install for 50+ repositories which is ridiculous (not to mention messy) given types make up less than 20% of our source code.

flow gen-flow-files to the rescue!
flow gen-flow-files solves all the above problems for us. We don't need to manage several special flow type repositories or add any extra work to our upgrade process. Nor do we need to double the size of our distributions. By adding this one line to our build script, flow types are released in the repository that defines them (under the correct version numbers and everything) and ONLY the flow types - nothing else. So simple!

If we had to resort to managing multiple flow type repositories to avoid doubling our distributions, THAT might be reason enough to get frustrated and abandon flow types altogether :(

@deepsweet
Copy link

I personally have almost zero examples when gen-flow-files works – it produces everything from bugs to syntax errors, especially with exports. In my opinion it should be either deprecated or much improved, but not left in its current confusing state.

@iddan
Copy link

iddan commented Mar 22, 2018

I think improvement is very possible and this tool is necessary. I use it (even though it produces bad syntax) for my alpha package reactive-material

@zaynv
Copy link

zaynv commented Mar 26, 2018

It's a shame because this feature is really important to have :(

Things like styled-components which is written in Flow doesn't even ship with type definitions, and flow-typed is always very behind in adding libdefs for new version of it (v3 still isn't in flow-typed).

Also, the new Pinterest library written in Flow needs to resort to weird hacks like this in order to ship flowtypes.

@TrySound
Copy link
Contributor

TrySound commented Mar 26, 2018

@zaynv this is not a hack. It's quite flexible way to provide as much as need entry points without a lot of duplications and ugly configs. Consider it as an official way to provide definitions from source.

Styled-components are probably had an issue I wasn't faced. They could use the same solution.

@zaynv
Copy link

zaynv commented Mar 26, 2018

Hmm, but even the creator of that Pinterest library referred to it as a hack. It's pretty weird to need to include the entire src directory in order to ship type definitions.

Also, I have a question about using types flow-typed? If you're only including files from src, the types you import from flow-typed definitions won't exist right? Will that make the types break?

@mgrip
Copy link

mgrip commented Nov 1, 2018

Any more discussion on this? I think this could definitely be a turn-off for people getting heavily invested in flow, especially for people who are active in publishing open-source modules. It seems pretty important for people to be able to easily and efficiently publish code written with flow types so that other projects using that code can get the types if they're also using flow, while still supporting other projects that aren't using flow. Typescript already has a solid solution for this, for comparison (basically the same as gen-flow-files).

@nmote
Copy link
Contributor

nmote commented Jan 18, 2019

We have some solid building blocks nearly done for a real replacement for this command. We understand that it's important for library authors who are using Flow to be able to publish libdefs.

We don't have an exact timeline, but I believe that this will be available within the next six months.

@polgfred
Copy link

Is that six-month timeline still pretty accurate?

@nmote
Copy link
Contributor

nmote commented Jul 2, 2019

@polgfred, sorry for the delay here. I've been waiting to see how #7750 plays out, but based on discussions in our Discord (feel free to join, by the way, we're better about responding quickly in there), I think there's no longer any reason to block on that issue.

Basically, there was some discussion about whether to prefer .js.flow files in flow-typed, or stick with the current approach where we use our libdef file format (which has a bunch of pitfalls). At this point, it seems like there is enough buy-in to support at least an option for using .js.flow files. And even if that doesn't pan out, it would still be useful for projects to be able to ship .js.flow files which describe only the module interface, rather than copying the entire source code as is done today.

I didn't want to spend time writing a gen-flow-files replacement that used .js.flow files which ultimately turned out to be useless with flow-typed, if they decided to stay with libdef files, and vice versa.

The the types-first project (which is what provides the building blocks I will use to generate .js.flow files) has matured significantly over the last six months. We recently enabled it on our largest codebase.

To directly answer your question, no, the six month timeline is not really accurate. I'll be on vacation for much of July, but I'd like to take this up after that. I'd like to find time to ship this in August or September.

@tajo
Copy link

tajo commented Mar 4, 2020

@nmote Is there any update on this? Are you planning a tool that would be able to generate libdef (declaration) files?

@nmote
Copy link
Contributor

nmote commented Mar 4, 2020

I believe @panagosg7 has spent some time looking at this.

The short answer is that yes, it's still something we intend to do, but we don't have a concrete timeline for it.

@chrisdothtml
Copy link

chrisdothtml commented May 5, 2020

Is flow dump-types a viable alternative to this? It seems pretty robust, but just needs something to convert the output into a libdef. Any reason not to use this for that purpose (e.g. if dump-types is also going to be removed soon)?

cc @nmote @TrySound

EDIT:
For context, we're looking into a working solution for using flow with yarn v2 pnp (which only works with libdefs)

@panagosg7
Copy link
Contributor

This is still something we're planning. We have some of the necessary utilities in place (see code under src/codemod/), but there's still some work to ship this under a gen-flow-files command.

The main disadvantage of dump-types is what you already pointed out: it will print types for every program location, not the parts we'd want in a libdef.

Another one is handing imported types; if a type is not defined in the file, then it won't add an import type statement for it. The utilities I mentioned earlier can handle this, along with other issues we've seen in similar settings.

@comp615
Copy link
Contributor

comp615 commented Mar 24, 2021

I've been looking into these for a few repos private and public, and trying to figure out what the story is around Flow and libraries. Building on @chixor's comment above. What I see as the main drawback is that typing is not always accurate using flow-typed defs. That requires a human intervention on someone's part to maintain and update those, which means the potential for errors.

One of the major benefits of co-located flow with the library dist itself is that when I go to upgrade it (as a user or maintainer), it will "just work" and accurately tell me which props are removed or added, without needing to reference the changelog and worry if the type-defs have been updated, or updated correctly...because it's pulling from the library source itself! (I recognize there may still be a need to run an extra pre-release build step)

I'm curious if Types First helps any with this proposition. My understanding is crude, but it sounds like it generates and caches signatures at export boundaries, which sounds pretty synonymous with what would be needed for a user of a library who's importing it. This seems like potentially a really big step forward for the eco-system overall; it gives maintainers a reason to use flow that they may not inherently have right now.

@nmote
Copy link
Contributor

nmote commented Mar 24, 2021

Yeah, so types-first can essentially generate interface files with the types only and no implementation, which is almost exactly what we want. However, it still includes imports and does not inline imported types, which means that it can't currently generate a single file to describe the type of a particular module entry point. This is probably fine, since you could still ship *.js.flow files that summarize the relevant types, and by excluding the body of files you'd mitigate (though not completely solve) issues with different Flow versions or config options. You'd also be adding files to your shipped library that are not actually needed at runtime, bloating its size somewhat.

flow-typed solves both of these issues by providing libdefs versioned by Flow version, and by making it so that consumers of your library pull down type definitions only during development, so you aren't shipping them to production environments too.

Anyway, back to the topic at hand -- I'd like to build something that can generate a single typedef with inlined imports, based on the signatures that types-first extracts, but we've all been busy and it hasn't bubbled up as a top priority yet. Another complication is that we are in the midst of a large architectural change to types-first that, among other things, changes how signatures are extracted. Instead of storing them as an AST (which had a lot of issues), we now have a custom type for signatures, but we haven't yet written something to convert that custom type back to an AST which would be required for this project.

@mondaychen
Copy link

Hi. I'm a co-maintainer of Recoil

As a flow-based lib, we've been replying on a third-party gen-flow-files to export flow types for users. However, that script is not actively maintained and now we are having difficulties upgrading to latest flow version. If we are not going to see an official support of publishing libdefs soon, any ideas how we can get unblocked on this?

@Brianzchen
Copy link
Contributor

Hey @mondaychen we talked about this before on recoil and settled on gen-flow-files. Would you be open to swapping out to flow-copy-source instead? It takes source code and rewrites it as a .js.flow file but works seamlessly, I ship many flow libs internally with this tool with no trouble and generally it's my preference because there's no logic involved to get messy and require extra maintenance.

@mondaychen
Copy link

@Brianzchen Thanks. I believe it works very well, but the downside is that it will make our released npm package much larger. I'll discuss with team.

@SamChou19815
Copy link
Contributor

The command was killed a while ago

@SamChou19815 SamChou19815 closed this as not planned Won't fix, can't repro, duplicate, stale Aug 8, 2023
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