Block-level /*flow-disable*/ and /*flow-enable*/ #740
Comments
|
+1 |
|
+1 |
|
FYI, you can disable flow warnings using Example (taken from above link): // $FlowIgnore: suppressing this error
var x : string = 123; |
|
Closing as resolved. |
|
There is a default https://flow.org/en/docs/config/options/#toc-suppress-comment-regex |
|
This works, of course, but doesn't allow for block disabling, only line-by-line suppression. It can be a bit messy to add an ignore on each line in a block. |
|
This isn't a solution to the stated problem in the original post. Blocks enable-disable should work just like eslint. Example: This is hideous// $FlowFixMe
type Props = {|
// $FlowFixMe
onOrderInputChanged : Function,
// $FlowFixMe
personAssignmentId : string,
// $FlowFixMe
actionTypeId : PropTypeActionType,
// $FlowFixMe
orderSetId : string,
orderValue : Immutable.Map,
// $FlowFixMe
activityId : string,
order : Immutable.Map
|};This should be the correct solution/* flow-disable */
type Props = {|
onOrderInputChanged : Function,
personAssignmentId : string,
actionTypeId : PropTypeActionType,
orderSetId : string,
orderValue : Immutable.Map,
activityId : string,
order : Immutable.Map
|};
/* flow-enable */ |
|
Doesn't seem like this issue is resolved. People still seem to want this feature. |
jamesisaac
changed the title
|
Is there a PR related to this issue ? This feature is key in my opinion 🤔 |
|
Any news about this issue ? Block disabling would a great tool to progressively implement Flow in an existing codebase |
|
Any updates on this? It's crazy to put 19 $FlowFixMe in my 100-line code... |
|
bumpity bump on this |
|
My solution for this problem was to stop using Flow :^) |
|
4 years and no one has taken care of the numerous amount of people wanting this addition badly. Looks like they are pushing us to move away from Flow altogether :-( |
|
I don't know if anyone on the current Flow team is looped into this thread, and the high number of 👍s on intermediate comments would not show up in any form of GitHub issue filtering... so not sure this is really on their radar. /cc @nmote @jbrown215 |
|
Could be really great yes... |
|
$FlowFixMe has two problems - it only ignores one line and it ignores all types of error. I don't like the above enable disable proposal, since it'll rapidly become unclear which enable corresponds with which disable as code is refactored and merged. Maybe something like this would be better: Either added to some existing curly-brackets scope - function, if, etc., or to a https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/block added just for this purpose. You could say that you'd still have to match up { with }, but we have to do that all the time anyway, and indentation helps you out a lot. |
|
+1 |
|
+1 |
1 similar comment
|
+1 |
|
Everyone giving a +1 or thumbs up, please give an actual example and use-case. I may be able to point you to alternative solutions. For example for OP's example of using |
|
As for OP's example, it's not specific to that exact syntax, but for blocks of code where flow errors should be suppressed. The official workaround is super hacky and the sentiments of this issue reflect how much people don't like that approach. If FB would just say NO, not gonna do it, at least it would save tons of people dead-end googling better workarounds. |
But what are the use-cases for this? Specifically |
You are just trolling at this point, right? |
|
I don't understand why you would want to disable typing for a block of code. Internally we don't have any use-cases for this. We are unlikely to ever implement such a feature, but if you show what your use-cases are perhaps I can recommend an alternative. |
|
@gkz So you know, flow doesn't support some basic Javascript things, right? Like using Some of those things are maybe stage 3 and not accepted yet. But some of them are in ECMAScript for 10 years now. Like, you get it, a lot of things don't work. And I feel like the community in general gave up asking or arguing about those things, back in like October 2019, a good amount of people even sooner, because there's always guys who'll just tell you to use worse alternatives. "Use this and that", "just use It is what it is. But this issue, right here, it isn't a feature request. It would be a compromise. Obviously this issue was created back in 2015, with arguably other intentions. OP asked for "conditional" comments, I think they just wanted to use a non-standard file and non-standard stuff and then validate it with flow, by virtually "commenting out" stuff so it doesn't parse that part and eventually throw an error. Which is actually a nice idea though! And there are a couple of use-cases for it, two of which OP even mentioned in their post, so if you're asking for that, scroll up. But I feel like the core of this issue shifted to a different, even more useful thing, which would be the ability to just disable flow errors and warnings for a specific block of code. With the current state of flow, this isn't a small feature that "would be nice" or something, it's literally a must-have at this point. So there you have it @gkz. The use case of this is years of ignoring standard Javascript, years of denying real issues with Flow, like the inability to extend it with plugins and custom built-ins. There's a blog, but it feels out of touch, much needed features are often completely dismissed for months, often issues just get closed for no reason whatsoever with a useless comment attached to it, like it was the case with this one. Not a single release in the last couple of months correlated with anything people actually wanted and need. I'm really tired of arguing and writing comments here, and I don't think I'm the only one. And we all get it, "open source", "resources", "priorities", etc. and obviously no one is entitled to anything. But I think the ability to disable flow for a whole block of code would be one of those things people will actually stay for. If this (and many other equally important but dismissed issues) won't get implemented, not because there are no resources or no use-cases or because they are hard to implement, but because the maintainers "internally" "don't have any use-cases for this", than I would honestly suggest switching to a private repository. There are two key issues. The lack of communication, and the lack of communicating if the lack of communication is intentional. Seriously, like @ddonahoevibrenthealth suggested, at this point, just say yes or no. It's really unclear what the goals of Flow are currently. And it's useless opening issue after issue after issue if we don't even understand what your motives behind closing previous issues are. It seems like you don't really care what the users of flow need. And that's fine. If the only factor for determining if an issue is worth working on, is if you have use-cases for it internally, then please communicate that to us. But I still have hope and care about flow, and that's why I really recommend implementing this feature in some way. It wouldn't just be a much needed QoL addition, it would be a step in the right direction. It would show your community, that those 60-something thumbs-up emojis aren't just emojis for you. These are all real people, with real use-cases. Please don't this dismiss this. Thanks. |
|
I'm not exaggerating by the way. Below are some PRs and issues I just found again because I wanted to know about the status of
Keep in mind, some of the issues above are in the top 10 most popular issues. There's no way the maintainers didn't see them yet. They are actively ignoring them. From now on I will use this comment to add any similar issues I'll find in the future, because trust me, there are at least 10x of those listed above with the same story: OP asks for something, then maintainer says "it's not that easy because...", then OP suggests a reasonable solution, points out general inconsistences of types, or asks for an alternative, then nothing happens for months or it gets closed). Flag this comment as "unhelpful", do whatever you want with it. I don't care, and you surely can relate to how much exactly I don't care. |
|
@nnmrts Your dedication here is impressive, but don't spend too much effort fighting the good fight when usage statistics about Facebook's own products appears to directly influence the current state of their upkeep. Bout 4 years ago there was an emerging need for better JS typing and Facebook gave way to the masses who's usage stats more closely mirror a React + TypeScript combo, so minimal effort is now placed in upkeep of flow. Flow isn't technically dead, it's just in a permanent coma. https://www.npmtrends.com/flow-bin-vs-typescript-vs-react-vs-@angular/core |
|
Alright, while writing the above, I actually researched about Typescript again, and honestly, it seems like it changed a lot. Before that, I thought Typescript was way more time consuming and way too clumsy and all of that. And it probably was when I decided between Flow and Typescript some years ago. My main concern was that with Typescript I had to compile files while with Flow I just have to use babel-flow-strip-types and everything works. I'm aware that I'll always compile and bundle it anyways later, but Typescript always felt like an extra build step where things can go wrong. But it seems like Typescript kept improving the developer experience over the years while Flow was just sitting there doing absolutely nothing. Hm. |
|
Please stop spamming with 👍, github has a facility for that which doesn't send unwanted notifications. This has gotten incredibly off-topic, but as long as we're writings screeds I might as well inject my unsolicited opinion. Click here for ramblings on the nature of open source that will only surely feed the fire in this mess of an issue.Isn't this an open source project? Since when did an open source project have an obligation to make all potential users happy?I think this commenter has nailed it:
The maintainers of flow are more concerned about internal considerations to Facebook than they are with pleasing the community. Opening up the flow source is essentially just a courtesy, a way of giving back to the community. Facebook is not interested in cornering the market on typed JavaScript, I think they have made that more than clear. And that's fine. If you don't like the priorities of the maintainers of an open source project, you fork it or you find an alternative.
This sentiment is confusing to me. Why would removing access to this work be the solution to any of your problems? As far as I can tell the only lives that should improve are those of the maintainers.
Flow is not any more dead than it was when it was first released. It has regular releases, with many large sweeping changes coming in recently, especially around object rest and spread which unblocks many other issues. Developer experience continues to improve at a nice clip, especially around the newer LSP implementation. The important thing to remember is that flow is making progress at the pace considered adequate for Facebook's internal use case. What's more, it will continue to progress at roughly that pace for the foreseeable future given that Facebook has invested in flow to the tune of hundreds of thousands of files and many millions of lines of code. The only things that could change this are Facebook deciding that it actually wants to truly take on TypeScript in the open market (unlikely), or if Facebook decides to undertake a herculean migration away from flow. My team continues to use flow after weighing the pros, cons, and risks. The ergonomics of flow are not great (but continue to improve over time). The stricter soundness of flow means I'll be less likely to encounter runtime errors vs TypeScript given my use cases. There is a significant risk that flow may become unmaintained or go in weird directions in the future given that it is mostly at the whims of an internal team of a larger company (Facebook), but this risk is mitigated by the fact that it is actually pretty easy to transition to TypeScript at the scale we're concerned about. As for this issue, I might be able to see some cases where this would be useful. For example, I had a coworker write this recently: export const fullscreenElementProperties: () => $ReadOnlyArray<Element | null | void> = () => [
document.fullscreenElement,
// $FlowFixMe—Flow is not aware that these vendor prefixed properties exist in some environments
document.mozFullScreenElement,
// $FlowFixMe—Flow is not aware that these vendor prefixed properties exist in some environments
document.webkitFullscreenElement,
// $FlowFixMe—Flow is not aware that these vendor prefixed properties exist in some environments
document.msFullscreenElement,
];Obviously this isn't ideal. The alternative proposed by this issue would be: export const fullscreenElementProperties: () => $ReadOnlyArray<Element | null | void> = () => [
document.fullscreenElement,
/* flow-disable—Flow is not aware that these vendor prefixed properties exist in some environments */
document.mozFullScreenElement,
document.webkitFullscreenElement,
document.msFullscreenElement,
/* flow-enable */
];This appears cleaner. That being said, I have a few concerns about it:
export const fullscreenElementProperties: () => $ReadOnlyArray<Element | null | void> = () => [
document.fullscreenElement,
/* flow-disable—Flow is not aware that these vendor prefixed properties exist in some environments */
document.mozFullScreenElement,
document.webkitFullscreenElement,
docunent.msFullscreenElement,
// typo in "document" above breaks ms browsers
/* flow-enable */
];Obviously this is the same in both formulations, The point here is that in my experience I reject almost all uses of |
That's actually all I'm saying. Imho, they didn't make that clear at all. Where is that statement? Where is the "flow is actually only for us btw, use typescript" blogpost? I know that some facebook projects like jest switched to typescript, but that didn't automatically mean that Flow doesn't care about community needs and contributions anymore. Or did it? Look, I'm not stupid, I don't feel "entitled" to anything, I know what open-source is. But suddenly saying that Flow was meant to be that kind of pseudo-open thing, is just weird. After all those years, all those community contributions, suddenly Flow being open-source is a courtesy? It's "giving back to the community"? What?? To what community? But hey, maybe I misunderstood something, and from now on, I'll just assume Flow is just a window to internal Facebook workings and not an actual open-source repo in the "traditional" sense. However, we both know that Flow wasn't that kind of project two years ago. I don't and I shouldn't even care about the reasons behind it, but we all know, the main priorities and goals of Flow changed someday back then. And I guess it's time for me to accept that. To accept that maintainers don't need to tell their users about a direction change. That "open-source" can also mean to release a piece of code, accept and implement hundreds of PRs and issues for years, and then act like none of that ever happened and we should be oh so thankful for it being open-source. That one of the main reasons for making something open-source is to increase said "pace", to increase the usefulness of a project, but somehow suddenly that doesn't apply to this one. And I also know where my misunderstanding comes from. Because Facebook actually has a lot of great open-source projects out there. And they all get advertised the same way as Flow. And they all get handled like you would expect an open-source project with a big number of users to get handled. But somehow Flow is an exception to that. Flow apparently is only open-source to get users to find bugs and improvement options the Flow team couldn't find themselves, to then discuss internally what's worth fixing and implementing. Alright then. Have fun doing unpaid work for Facebook. |
Really not sure where you are getting this narrative from. Have a look through the Contributors list: https://github.com/facebook/flow/graphs/contributors - out of the top 30 contributors, 29/30 of them are/were Facebook employees. The only person who isn't is goodmind, whose contributions started in 2019 (well after the "2 years ago" you're saying this direction changed). Flow has always been a tool with development almost exclusively driven by Facebook.
Facebook have devoted a team of engineers, for 5 years, to implementing a system which brings advanced types and soundness to JS codebases, to improve developer productivity. This was unheard of before Flow (TypeScript at the time didn't come close), and the fact that TS has converged towards this direction over the years is probably in a large part thanks to influence from Flow. Facebook could clearly succeed just as easily in producing this tool behind closed doors. But luckily for us they've made it open source and accessible to anyone, and are receptive in helping resolve crashes/bugs etc encountered by users outside of FB. Yes it would be nice if they placed even more emphasis on the priorities of the community, as Microsoft are doing with TS, but the reason for that is clear... MS is a software company with a large portion of their business model being software development tools for developers. Of course they are investing heavily in trying to monopolise the open source market into their own ecosystem (see acquisitions of GitHub and NPM). On the other hand, Facebook is a social network which makes all its revenue from advertising. Even if they poured immense resources into trying to compete with MS on this front, how would it benefit them? It's great that FB have made this tool available to the masses. This has been of great benefit to the JS community as a whole - look how many projects are written in TS these days, with many features inspired by Flow. For your own usage, you should weigh up its pros and cons against TS, and decide which one you want to use. No one is forcing you to use Flow, and certainly the Flow team is under no obligation to steer the product in the direction you'd like, if it doesn't align with their own priorities. |
|
@nnmrts You've established two possible extremes, where either flow is an altruistic open source endeavor beholden to the community, or an opportunistic racket to fleece the naive community for free contributions. The reality is, as usual, somewhere in between. |
@gkz #8346 is one I have which could be solved by this (already linked previously) |
|
Here is another use-case test('Should render membership table', async () => {
let wrapper;
await act(async () => {
wrapper = await mountWithMocks(
// $FlowFixMe
<OrgDetailsView
{...{orgDetails: orgData, memberships, uuid, userEmail}}
/>
);
});
// $FlowFixMe
expect(wrapper.text()).toMatchSnapshot();
// $FlowFixMe
expect(wrapper.find('table')).toHaveLength(1);
}); |
|
A use case for me is when I use Ramda's pathOr I get an error |
|
@jamesisaac Though you're right that the flow team and facebook owe nothing to any of us and I truly am grateful there is such a tool for me to use. But I tend to agree more with @nnmrts that something is not quite right with the way things are communicated to the public.
This is quite eye opening, given that jest moved to TS to support the developers that were actively working on the project. and React is working on rewriting their entire documentation site for the sake of helping new developers coming into react given that hooks > classes these days. I don't think either of these were/are simple tasks, nor does it benefit facebook in their crusade to earn more ad revenue. Writing clear documentation isn't easy, it takes planning, meetings, and of course writing with lots of reviewers to ensure it makes sense to range of users. Now I don't know what the size of react team is vs flow team, maybe there's just less funding. But personally I'd imagine a type system that reduces bugs would have more funding than a view library that implements the building blocks. What I'm trying to say is that there's more to a frontend than just a view lib and react seems to have massive emphasis to serve others needs outside of what facebook internally needs. If anything, making it easier to submit type def and documentation changes would be a large step in the right direction given that flow defs are still incomplete (maybe these are types that facebook never uses). Right now I've opened a few and I don't even know where to ask to get them reviewed, I just see the list piling up. It would be nice to at least know if someone read it, found that it wasn't a good PR, it was missing something, and what could be changed to satisfy requirements. But back on topic, my use case is the same as @robatwilliams I have a single large file that exports a list of function, I can't afford to type them all at once but some are deemed more critical and it would be nice if I could type them incrementally. |
That move was made after the project was handed over to the community, by a lead maintainer who was (as far as I'm aware) never a Facebook employee (source)
Surprising perspective.... Flow is a standalone piece of of build-time tooling, with all remnants stripped out during transpilation. Where it works, that's great, and it can help identify bugs -- where it doesn't, it just means you need to be a bit more careful. No external tooling really builds on top of it, as evidenced by the regular breaking changes (multiple breaking releases per month), and lack of interest in supporting 3rd party typedefs. On the other hand, React code is delivered to billions on end-users, and is the foundation on which an enormous amount of 3rd party libraries (64k dependents according to NPM), and Facebook's own libraries (React Native, Relay, DraftJS, etc) build on top of. So making sure it's being used appropriately is critical for that ecosystem to function, so the many layers of libraries integrate well together. They took 3 years to make a major breaking release. React is quite an outlier in its popularity among FB's open source projects, but even then, it seems to have only just recently reached the point where FB has brought someone onto the team focused on education & community engagement. It's nice they've done that, but I don't see how this now means we should expect they do the same for every open source project they run... |
|
@jamesisaac I see where you're coming from, React has gotten so large and so many people depend on it that it's a make or break library at this point. But also, Flow is allowed to make constant breaking changes because as you say, they are a build tool that gets stripped out, it's not runnable code. Constant breaking changes indicates a constantly improving type system, and because it's not like TS which is a compiled tool if you can't fix your code then you and just put a suppression comment which this issue is all about.
I don't think that's necessarily true, yes they may have brought on someone which I'm not aware of but majority of the core react team as super active on social media and I think that really helps to push a tool forward. When you have a tool no matter how great or limited it may be, if you have core members who will actively reply and help you you'll feel less of the limitations and you won't have expectations shot down. Though that's a big ask and no one should be forced to help everyone just cause someone decided their project is open sourced 🤷♂️
Well that's just referring to |
|
Now that we have specific Suppression Annotations, the request here is much safer and should be reconsidered. Similar use case pointed out above, the annotations break too easily if a linter (or someone) converts a single line of code into multi-line rendition:
// $FlowFixMe[signature-verification-failure] shed happens...
const Quiet = (a,b,c,d,e,f) => {
// $FlowFixMe[signature-verification-failure] apply to all args and return? wrong...
const Quiet = (
a,
b,
c,
d,
e,
f
) => {So block comments would really help stay consistent with javascript multi-line expressions, not to mention several other benefits listed above (avoid repetitive declaration of the same annotation for typing sake, etc.). |
|
As per our post on Flow's direction: https://medium.com/flow-type/clarity-on-flows-direction-and-open-source-engagement-e721a4eb4d8b |
|
Flow's opt-in / opt-out architecture is the only reason I'd use it. Otherwise for strictness why not typescript, scalajs, etc? |
You have, here, defined FLOW as an academic exercise, not an engineering project for real-world use. |
|
@keithgabryelski Actually the opposite is true, this direction comes from the real world requests from our users of Flow at Meta. They have asked us for increased type safety as that is important to them and the very large real-world JS projects they are responsible for (and which billions of people use). |
I'm glad to hear you are taking input from a large customer base. That, however, does not preclude you from having a block-level disable. I am not looking for a fight, here -- in fact, I'm looking for help so I don't have to spend the time ripping out my company's 6 year investment in the use of flow.
and the ability to accept what your team has given us while admitting that certain areas of code are just not a fit for FLOW just yet. these are my opinions ... not of any employer i might have |
|
My recommendation, if you have large chunks of code you cannot type, is to pull those out into their own modules, and then not enable |
|
jesus. thanks for the absolutely obvious answer |
|
For your wish list
What version of flow are you on? I would say from version 100 and up only two changes have been difficult to fix and therefore worth hold off on upgrading, the one with types first dropping classic mode and the one that had
It'll be tough to get contributors in projects that don't use flow to support flow, the alternative and honestly a really good solution is to instead rely on flow-typed and as a shameless plug I've been adding lots of definitions recently, is there anything that's missing for you right now that you really wish was there? Unfortunately I only type simple things I come across or libs I actually use and can reliably know it works.
I can't say for sure but I've seen the release notes talk about improvements in performance and from my perspective speed's been just fine so maybe it's related to being on an older version? |
I've taken the three days to think about this answer and believe it to be too harsh for any response that is trying to be helpful. I'm sorry for it. My response should have given you facts I believe are not being considered. Separating out FLOW code that is not easy to convert into a separate source file is, well... a solid solution -- especially if you are writing new code; files But for code that has been inherited or is being converted from one version of flow to another, it is easier for everyone (coder, reviewer and maintainer) to keep the code where it lay. The least number of contortions allows for provenance to more readily be ascertained. So, there is a natural pull (for my teams) to use FlowFixMe multiple times instead of moving the code out of the file. In addition we have a natural pull to to not use FlowFixMe at all, rather spending the time to update the code-base in whole -- this may be issues with OCD and germaphobia but it has caused us to hold off upgrading to newer versions of flow until we can spend the time to clean all code completely. This prevents any incremental solution, which would allow us to get used to the latest in flow and fix things in smaller batches... this spreads the burden of conversion over time and across coders, it also reduces the burden of code-reviewers from having to deal with large PRs that are basically equivalent to my kids pushing food around their plates then claiming they have finished their dinner. (PR reviews are currently a bottle neck in our development process. PR reviews of 40k lines of code (a small portion of our code base) are painful even if they are just completing our typing and mostly programatic). Be assured I am grateful for the security flow has provided our projects -- but I'm not sure I could justify using flow in another large project without some solutions to the usability issues I mentioned. To sum up: flow's typing system, like GIT, Jenkins and React, is just one part of our development process -- where it impedes our progress we naturally consider if alternatives can meet our needs without the RIDE OR DIE mentality. |
and others
Will something like conditional comments make sense for Flow? There are some use cases (custom template inlines [1] or #718 [2]) when stripping some lines will allow Flow to understand the file.
Basic implementation I tested:
[1]
[2]
The text was updated successfully, but these errors were encountered: