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

Flow Enums: Syntax #7837

Open
gkz opened this issue Jun 19, 2019 · 11 comments
Labels

Comments

@gkz
Copy link
Contributor

@gkz gkz commented Jun 19, 2019

Flow Enums: Syntax

Flow is working on adding an enums language feature, for improved developer experience, performance, and safety. [EDIT: This would be an optional, opt-in feature, we would not change the default of not requiring any transformations other than stripping types]

Here, we examine the proposed syntax and grammar/ast specification for this feature (but do not examine runtime semantics or type-checking behavior, which are in development and will be shared at a later point).

For most up to date content, see: https://github.com/gkz/enums

@goodmind

This comment has been minimized.

Copy link
Collaborator

@goodmind goodmind commented Jun 19, 2019

Please no, enums aren't standard js.
This is why I don't use TypeScript with their enum and namespace.

Adding enum would rob us if it would be added to js as tc39 proposal.
Also how Flow would support this if it only removes types?
Please don't add any runtime features if you have plans for more

@gkz

This comment has been minimized.

Copy link
Contributor Author

@gkz gkz commented Jun 19, 2019

To be clear, this would be an optional, opt-in feature, we would not change the default of not requiring any transformations other than stripping types.

@goodmind

This comment has been minimized.

Copy link
Collaborator

@goodmind goodmind commented Jun 19, 2019

@gkz this would still be added to babel and reserve enum forever for Flow, denying any TC39 proposal with enum

@XaveScor

This comment has been minimized.

Copy link

@XaveScor XaveScor commented Jun 19, 2019

I think flow can grab this idea for enums. https://docs.python.org/3/library/enum.html

class AEnum extends Enum {
  A = 1
  B = 2
}
@PinkaminaDianePie

This comment has been minimized.

Copy link

@PinkaminaDianePie PinkaminaDianePie commented Jun 19, 2019

I'm generally against this proposal.

  1. If we check how TS approached compiling and long fight of the community for babel plugin, we can see that users still prefer to have only type checker, even if MS declare TS as a new language. People don't want to add any new semantic, they just want normal JS with types. Flow is a type system, not a compiler/language etc. It would be nice if it would stay the same and devs would be focused on soundness instead of code performance. Flow can increase code performance by improving the type system itself and make type inference better - there is a lot of cases when refinement invalidations happen or refinements are refinements are not precise enough - it makes developers add extra checks which could be unnecessary. Quite often we need to add some code just to "make Flow happy" instead of dealing with real code issues. It takes time, it makes code less performant. Improving in such areas will help us even more, but would still leave Flow as a type system, not as sort of compiler.
  2. Ad-hoc like improvements without a visible strategy. How does Flow want to evolve? Do you wanna add single runtime feature? Set of features? Move away from JS semantic to something different? Where is discussions/RFCs without that? Such a feature not only adding breaking change from code perspective but can change what Flow actually is. Such decisions should be carefully considered and discussed, but for not it looks like ad-hoc. It will change what Flow is. First RFC in such case should be not about enum itself, but about the scope of runtime features, themselves. There is no sense to discuss any runtime features without any discussions should Flow have runtime or not and if should - how should it be addressed.
  3. Flow syntax was easy to parse and a lot of tools adopted it. The more new syntax will be introduced, the harder it will be to keep up with it. Some old tools drop support of Flow because of its decreasing popularity, some just leave support as is. But as for new tools, people don't want to support Flow even if it's easy. Here is an example of compiler https://github.com/swc-project/swc written in Rust, which is much faster than Babel. They do support TS but don't wanna deal with Flow. The more new syntax you will add, the harder it will be for the community. Flow popularity decreases and any breaking changes will do it even faster. Just remember the story with explicit types for exports in v0.85 - it makes Flow faster but also forced to A LOT OF companies drop Flow. Please think from that perspective as well. Who will add new syntax to Babel? To ESlint? To IDE code highlighting? Sonar Qube and other static analyzers? To hundreds of tools which people use. If these tools won't add your new features, people will, again and again, face a choice "drop my favorite tool to make Flow happy or switch to TS". Don't think that the community would address these issues by themselves - Flow community decreased a lot and people afraid to contribute to the tool which loses popularity. New features in Flow without their support in other tools will just force more people to switch to TS.

I do understand that runtime features can improve dev experience, but please don't think about that in isolation. Think about where Flow is, about other tools which work together with it, about community, etc. Currently, such an approach for runtime will make things worse, not better. People in 3rd party tools won't add such feature and people will either drop these tools (with loss of DX) or will move to TS. Any of these ways won't make them happy about it.

@gkz

This comment has been minimized.

Copy link
Contributor Author

@gkz gkz commented Jun 19, 2019

The plan is to make this a TC39 proposal, so this will be no different than other early proposals Flow has chosen to support, other than the fact that this one is coming from the Flow team itself vs. other teams at FB. That is, I hope in the future for this to be a feature of JS itself. After I finish defining and iterating on the runtime semantics, I will create and share this proposal.

Please let me know if you have any feedback on the content of the proposal itself :)

@zeorin

This comment has been minimized.

Copy link

@zeorin zeorin commented Jun 20, 2019

Sounds like the TC39 proposal should come first, then.

@PinkaminaDianePie makes some good points.

If Flow is still intended not to require any transpilation other than stripping annotations, then there ought to be e.g. a Babel plugin that will do the required transpilation for the TC39 proposal.

If it is instead built into Flow itself, what happens to this feature if the TC39 proposal goes nowhere? Will Facebook's Flow team release a codemod to change all the code that used this feature back into standard JS+Flow?

I think the issue here is that there hasn't been much context provided. The proposal seems fairly technical, jumping into the AST terms that would be added, etc., but not really talking about the process, what happens if the planned matching TC39 proposal doesn't succeed (which was not revealed in this initial proposal), etc.

You say that it's optional, opt-in. However, if this becomes a Flow "language" feature, it's only optional for your own code. Any 3rd party code may or may not end up using these features, and we'd have to contend with them. It's about as "optional" as using classes in JS is. They're just part of the language. There isn't a valid version of JS with classes, and and equally valid version of JS that doesn't have them. I feel like calling this "optional" is a poor cop-out for the lack of community engagement from the Flow team.

@goodmind

This comment has been minimized.

Copy link
Collaborator

@goodmind goodmind commented Jun 20, 2019

Yeah, if libdefs would use this syntax then it is no longer optional... and if it doesn't go through TC39 then it would be same thing as decorators all over again

@noppa

This comment has been minimized.

Copy link

@noppa noppa commented Jun 20, 2019

To comment the actual feature, whether it's an EcmaScript or a Flow feature - I don't see enums as such a useful feature even when I'm using TypeScript, since string literal union types pretty much solve the same problem.

I guess I just don't see how

setTheme(Theme.DARK)

is any better than

setTheme('DARK')

as long as the string literal is type checked against union type like

type Theme = 'LIGHT' | 'DARK'
@villesau

This comment has been minimized.

Copy link
Collaborator

@villesau villesau commented Jun 23, 2019

Yeah, if libdefs would use this syntax then it is no longer optional... and if it doesn't go through TC39 then it would be same thing as decorators all over again

We could have a lint in flow-typed preventing using enum syntax in libdefs. And there shouldn't be much need for it anyways: Libdefs are usually written for standard JS (or TS in some cases) and since standard JS does not have enums, libdefs should not need to have them either.

So in case of libdefs such syntax does not matter if we lint against it.

On a personal note, I also definitely think that TC39 proposal should be filed as soon as possible to mitigate the fear of Flow not being just type system anymore.

Anyways, in overall enums would be super nice feature!

@dnalborczyk

This comment has been minimized.

Copy link
Contributor

@dnalborczyk dnalborczyk commented Jun 26, 2019

I'd love to have enum types in javascript! but I agree, an official TC 39 proposal as a first step would be better than a backdoor strategy through flow.

@gkz not sure if you know, but there have been some (yet unofficial) attempts for enums, both are from current TC 39 members and unfortunately both haven't gone forward with the proposal yet.

https://github.com/rwaldron/proposal-enum-definitions

https://github.com/rbuckton/proposal-enum

@goodmind goodmind referenced this issue Jul 7, 2019
3 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.