Flow: Type annotations inside destructuring

Created on 27 Jan 2015  ·  113Comments  ·  Source: facebook/flow

It seems like the following should be supported:

var [a: String, b: Number] = ["foo", 1];

function foo([a: String, b: Number]) {}
foo(["foo", 1]);

Is this a deliberate decision or was the scope of ES6 type annotations restricted until ES6 became more widespread?

Thanks!

destructuring feature request

Most helpful comment

@RReverser You can just do:

class A {}
var { a }: { a: A } = obj;

All 113 comments

We've held off on adding inline type annotation for destructuring since there isn't an obvious place in the grammar for object destructuring (var {a: string, b: number} = x will bind x.a to string and x.b to number). Array destructuring doesn't have this problem, so we could definitely add it.

Also I don't think TypeScript supports these kinds of annotations, so it wasn't something we urgently needed to support. You're saying the 6to5 acorn fork has support?

@gabelevi Yeah, I was rewriting some of the flow parsing in acorn-6to5 and _accidentally_ added it in. Only my examples with array destructuring work correctly because as you said, syntax ambiguity.

Hmm, interesting how TypeScript / Flow are going to resolve this. Ideally, we could just do it during scope analyzing so if "property value" of object pattern is linked to type, then use it as type annotation and if not, use as destructuring target.

The only downside will be for cases like:

class A {}
var {a: A} = obj;

when you really want to override locally defined class and reassign A = obj.a, but I don't think this is an often case and it can be easily workarounded on developer side.

@RReverser You can just do:

class A {}
var { a }: { a: A } = obj;

@sebmck That doesn't look good enough - variables and their types are split into different sides of operator. It works for regular object literals, but doesn't look so nice for bindings.

I've been thinking about this for awhile, and I think the : type paradigm just isn't going to fit into the grammar for object literals, patterns and types. I think if we're going to add inline type annotations then we need to switch paradigms. I've been thinking maybe we could do something like

// Works well for object types
var { number x; string y } = { x: 123, y: "hello" };
// Works well for object patterns
function foo({number a, string b}) {}

// Not so sure if object literals need this
var z = {
  ?string m: null,
  ?number n: null
};

// because soon Basil's new expression annotation syntax will land 
// which will help object literals
var z2 = {
  m: (null: ?string),
  n: (null: ?number)
}

The downsides include that this paradigm doesn't match the other type annotations and that there are certain reserved keywords in the modifier position (get, set, static).

@gabelevi, maybe I am being dense here, but why is var {a: string, b: number} = x not possible? Shouldn't the context be enough to disambiguate between destructuring-with-type-annotations and object literal syntax?

Oh, nevermind. I forgot about rebinding destructuring.

TypeScript uses as as a coercion operator. How about the following?

var { x as number, y as string } = { x: 123, y: "hello" };

function foo({a as number, b as string}) {}

var z = {
  ?m: null as string,
  ?n: null as number,
};

Alternatively, one could also use “Basil's new expression annotation syntax” everywhere:

var { (x : number), (y : string) } = { x: 123, y: "hello" };

function foo({ (a : number), (b : string)}) {}

var z = {
  m: (null : ?string),
  n: (null : ?number)
}

But I like as, because then there is no overloading of the single colon (and less parens). A double colon (instead of as) would also be nice, but IIRC that is reserved by TC39 for guards.

Hmm, I actually like { (x : number) } = { x: 123 }. Seems totally unambiguous and unlikely to conflict with future ECMA-262 proposals. Any objections? /cc @gabelevi

:+1: to @rauschma's idea of using "as".

:+1: to @rauschma's other idea of using "expression annotation syntax", or what I naïvely call "casting".

This would be super useful for functional React components:

const Component = ({
  propA,
  propB,
}) =>
  ....

Currently, the best way is to create a type and access the props via field access (that way only prop. is repeated a lot, but not the names of the props).

type Props = {
  propA: SomeType,
  propB: SomeType,
}
const Component = (props: Props) =>
  ....

Clearly the destructuring and typing syntaxes clash :(. This makes sense to me, but is super ugly:

const Component = ({...}: {
  propA: SomeType,
  propB: SomeType,
}) =>

(since empty {} destructuring seems to be going through the committee, and wouldn't be obvious anyway).
This is probably currently illegal, but I have no reasoning for it:

const Component = ({{
  propA: SomeType,
  propB: SomeType,
}}) =>

What I'm looking for is a solution that lets us easily go from typed params to named typed params:

f = (a: X, b: Y) => ...
f = ({{a: X, b: Y}}) => ...

What about this? I like "as" operator too - it is short, simple and evident. This example is good:

function foo({ a as number, b as string }) {}

Just bumped into this myself. I also support { (x : number) } = { x: 123 } since it aligns with type annotations elsewhere. I don't think we need to add a new keyword.

For some reason I thought { x: (localX: number) } = { x: 123 } would work, but perhaps it also should work.

This syntax is confuzy - too easy to make mistake.

I was on a long plane ride this weekend so I figured I'd toy around with the paren-grouping syntax (i.e. let { (x:number) } = {x:123}) because it's simple, it composes well with the rest of of our lval type annotations, and when paired with object literals it unlocks the ability to type a property slot (rather than having to type the value and inferring the property slot from that). On objects, this would unlock the ability to understand deferred initialization of a property similar to how we can with let/var (i.e. let foo:number; foo = 42; --> let a = { (someNum:number) }; a.someNum = 42;)

On syntax: I went with what matches the most with our other annotation positions. TBH I don't find the syntax beautiful, but I don't think we're going to find something that everyone agrees on there. Additionally, after playing a bit I don't find it prohibitively inexcusable.

My branch is linked below. It doesn't wire up enforcement of the destructuring annotations just yet -- but it shows that we _can_ if we wanted to pursue this route:

https://github.com/jeffmo/flow/tree/pattern_lval_annotations

Did you get in contact with the TypeScript team? Ideally, a syntactic solution would be adopted by both Flow and TypeScript.

@rauschma: I haven't on this, no (this was just plane-ride hackery) -- I'm happy to reach out.

EDIT: I guess this isn't really casting, so maybe casting syntax consistency doesn't matter too much

@jeffmo It's not about casting consistency, but rather would be nice to have as much syntax shared as possible. I'm pretty sure they were also exploring options for typing destructuring, so collaboration could help in both short-term and long-term perspectives.

Also similar syntax in TS and Flow is good for IDE.

Indeed. I will be sure we follow up with them before moving forward with anything here

👍

I know that you probably can't switch the whole syntax over now, but it's too bad that we can't have this:

Regular function call

function (
  number id,
  string name
) {
  // function body
}

Object destructuring

function ({
  number id,
  string name
}) {
  // function body
}

Array destructuring

function ([
  number id,
  string name
]) {
  // function body
}

One of the most useful places to have types is in function signatures, so it's too bad that Flow doesn't support a cohesive and readable syntax there.

@jtremback It's not too bad at all. Flow does indeed support a cohesive and readable syntax. Destructuring aside (I'll address it below), your

function (
  number id,
  string name
) {
  // function body
}

is just (by the way a function is an arrow type, you don't have a full function type there) a personal preference over

function (
  id: number,
  name: string
): any {
  // function body
}

I prefer the latter, you prefer the former. It's not too bad.

Destructuring actually doesn't mean that your function's argument is what you wrote it is. I'm talking about the object destructuring. Your array destructuring is actually a tuple type which is [id, name]: [number, string].
The main point is — you argument is an object or a tuple. It has a specific type. And this type would be the argument type. Just because you're destructuring it the type doesn't change. So I actually prefer to declare my type elsewhere (normally you would use it in several places anyway) and then just do { id, name }: MyType. Flow will figure the rest up. The trick is to use Flow's impressive mechanisms not to try to annotate every single thing in your code.
But it's just my opinion. However the presence of one invalidates your too bad argument.

@alexeygolev What I mean is that the use of : in Flow's syntax collides with the extensive use of : in javascript. Object destructuring allows us to have named arguments in javascript, which is pretty great for readability and reduces accidental bugs. Unfortunately, the : makes this tricky syntactically to use with Flow. I think that @jeffmo's solution is a good compromise though.

Has there been any progress on this recently?

I also am in favour of @rauschma's suggestion:

var { (x : number), (y : string) } = { x: 123, y: "hello" };

function foo({ (a : number), (b : string)}) {}

var z = {
  m: (null : ?string),
  n: (null : ?number)
}

I'm very interested in this working as well since it would drastically simplify stateless functional component definitions in React. Currently those have to look like this:

// @flow

import React from 'react'

type Props = {
  name: string
}

const GoodMorning = ( { name }: Props ): React.Element<any> =>
  <div>
    Good Morning, { name }
  </div>

export default GoodMorning

Rather than this:

// @flow

import React from 'react'

const GoodMorning = ( { (name: string) } ): React.Element<any> =>
  <div>
    Good Morning, { name }
  </div>

export default GoodMorning

The second example is much terser and DRYer, and this is a pain point we see every day in our React codebases. (We do separate type aliases because inline within the signature gets ugly with more than 1-2 properties.)

The advantage of the paren-grouping syntax is that it's fairly intuitive since it reuses existing syntax in a way that makes sense. An inline type annotation "feels" like an expression, so "disambiguating" it with parentheses within a destructuring expression isn't too surprising.

The { (name: string) } solution looks great. Is anyone working on getting it implemented?

The last I heard from any member was above where @jeffmo said they'd try to coordinate on this with the Typescript team to make sure there isn't unnecessary divergence. If we get consensus on this I might like to get my feet wet trying to make a PR - I don't know if it would be wasted work though?

@jrajav
Realistically:

const MyComponent = ({
  ( aProp: string ),
  ( someOther: number ),
  ( zethIs: Object ),
  ( onClick: (event: Event) => void ),
  ( onSubmit: (event: Event) => void ),
}) =>
  ... {onClick} ...

I'd probably stick with the current:

const MyComponent = (props: {
  aProp: string,
  someOther: number,
  zethIs: Object,
  onClick: (event: Event) => void,
  onSubmit: (event: Event) => void,
}) =>
  ... {props.onClick} ...

But that syntax could still be useful elsewhere.

@xixixao I'm confused - doesn't your second example still conflict with native object destructuring syntax?

@jrajav The second example is actually the current syntax. The parameter is not destructured, simply annotated with a separate object expression.

@ajhyndman Ah! sorry - I missed the top line. Yes, that does work currently but is clunky when you are using actual destructuring in the signature itself (which is all over the place in a modern React app) - it results in a lot of repetition. I think it's far from ideal.

const MyComponent = ({
  aProp,
  someOther,
  zethIs,
  onClick,
  onSubmit
}: {
  aProp: string,
  someOther: number,
  zethIs: Object,
  onClick: (event: Event) => void,
  onSubmit: (event: Event) => void,
}) =>

@jrajav My point was that if the syntax required parens around each field, I'd prefer the current pattern that doesn't use destructuring at all, which would kind of go against the whole point of coming up with this syntax :/.

I'm not sure how it defeats the point, if the point is to make annotation terser and reduce repetition. Can you elaborate on that?

I can see how one might think it's a bit ugly but I think it might be the best choice at hand, regardless.

I'm just spitballing... We wouldn't have any of these problems if Flow's type syntax didn't overlap with the ECMAScript syntax for destructuring aliasing.

If, for instance, Flow had borrowed the type declaration syntax from Haskell, we'd be able to do this:

1.

const MyComponent = ({
  aProp:: string: anAlias,
  someOther:: number,
  zethIs:: Object,
  onClick:: (event:: Event) => void,
  onSubmit:: (event:: Event) => void,
}) =>
  ... {onClick} ...

Or, if ECMAScript had opted for the same alias notation in destructuring as with named imports:

2.

const MyComponent = ({
  aProp: string as anAlias,
  someOther: number,
  zethIs: Object,
  onClick: (event: Event) => void,
  onSubmit: (event: Event) => void,
}) =>
  ... {onClick} ...

What about leaning on the ES6 computed property names syntax?

3.

const MyComponent = ({
  [aProp: string]: anAlias,
  [someOther: number],
  [zethIs: Object],
  [onClick: (event: Event) => void],
  [onSubmit: (event: Event) => void],
}) =>
  ... {onClick} ...


const obj = {
  [aProp: string]: 'string',
  [aMethod: Function]: () => {
    ...
  },
};

The only thing I'm not certain of is whether or not that's ever going to be ambiguous with a preceding expression in objects. I guess it could also lead you to the mistaken assumption that you can use an expression to resolve the name of the key you wish to destructure from a parameter.

[someProp: string]: 'hello' is not the same as someProp: 'hello'. It's same as [someProp]: 'hello'. Also it makes it look like you're specifying the type of the key, not the value.

Does someone work on getting those things @ajhyndman done? It is something which makes flow a bit lagging.

We are used to write this:

export default ({ isActive }) => (
  <MyAwesomeComponent isActive={isActive} />
);

So how can I set isActive type?

Or even worse, if I set isActive alias this way: ({ isActive: active }) it is correct ES2015+ syntax. Seems like I am unable to use it with flow :(

For your example, I'd just do:

export default ({ isActive }: { isActive: boolean }) => (
  <MyAwesomeComponent isActive={isActive} />
);

Which is very reasonable for annotating a single prop. Components often have many props, however, at which point we use:

type Props = {
  isActive: boolean,
  className: string,
  label: string,
};
export default ({ isActive, className, label }: Props) => (
  <MyAwesomeComponent isActive={isActive} className={className}>
    { label }
  </MyAwesomeComponent>
);

Yes, exactly this way I use flow with react, but what about function aliases, how should I use them with flow?

e.g.:

const doLog = ({ veryLongPropertyWhichICannotChange: prop }) => {
  console.log(prop);
}
doLog({ veryLongPropertyWhichICannotChange: 1 }) // 1
type P = { veryLongPropertyWhichICannotChange: number };
const doLog = ({ veryLongPropertyWhichICannotChange: prop }: P) => {
  console.log(prop);
}
doLog({ veryLongPropertyWhichICannotChange: 1 }) // 1

Above code sample in flow repl

@acusti awesome! Thanks!

I acknowledge that I'm _very_ late to this party, but is there any value in leveraging the assumption that a Flow type-comment will always be a type assertion?

For instance, I feel like this is pretty unambiguous:

function foo({
  x/*: string */: inputXValue,
  y/*: number */: inputYValue,
}) {
  // ...
}

I haven't read the code that reads in the type-comments, so if the type assertion in the comment is transparently added to the AST at read time, then it might require too much work to implement. The syntax isn't perfect, especially not if you're used to using Flow types as a first-class part of the language, but I figure it's always better to voice an idea.

I just hit this today in my first 5 minutes with Flow -- I went over-eager on adding : type and put it in an object destructuring in a function call too. Although the first use will get an error, why doesn't destructuring (or any assignment) to a type name get a Flow warning immediately?

(This newbie likes jeffmo's proposal too.)

What about this:

const { (a: AType): aalias, (b: BType): balias, (c: CType), e: ealias } = e;

@akomm It does seem like that syntax has the most support. I believe it has been suggested above a few times already. 👍

https://github.com/facebook/flow/issues/235#issuecomment-174792066

lol I must have missed it! :D

Just to add my 5 cents...
For me this issue is pretty important, because destructuring is the only way to simulate passing named arguments in javascript. And, because of this issue, function definitions look ugly and unreadable and drastically increase code verbosity. Declaring type information separately from the function definition doesn't help too much because it doesn't eliminate verbosity and repetition. So for now the only option I see is to use object or some other weak type, and it makes me unhappy :(

It would be awesome if we could somehow find a solution to this issue in the foreseeable future.

New to Flow and testing it out a bit for a project. Really liking it so far 👍! I think I'm running into something that might be related to things discussed in this issue, but not sure yet.

I usually try to avoid typing arguments inline, but I'm one of those who likes to separate types from my code where possible... personal preference :). Using functional type signatures, I can get some nice usage out of Flow so that component instances are created with expected props wherever they are used.

However, I noticed a difference with destructured variable type checking when using a functional type signature vs inlined arg and return types (v0.48.0). I'm seeing a difference in behavior with these approaches, and wondering if these two examples should both behave similarly and be caught by the type checker (only "Example 2" using inlined Props type will catch the invalid default prop value). This may be the expected behavior (since I'm not explicitly typing the arg, only the function in "Example 1"), but just want to know for sure :)

// @flow
import React from 'react'
import type { Element } from 'react'

// generic type signature for functional React component
type ComponentFn<P: {}> = (props: P) => Element<*>

// define props
type Props = {
  foo: string,
  bar?: boolean,
}

// using function type
const MyComponent1: ComponentFn<Props> = ({
  foo,
  bar = 'false', // <-- invalid default prop
}) => (
  <div>{bar && <p>{foo}</p>}</div>
  // explicitly casting `bar` here with `(bar: boolean)` would catch it
)

// No Error when using invalid default prop `bar`
const test1 = <MyComponent1 foo="test" />

// using inline argument & return value types
const MyComponent2 = ({
  foo,
  bar = 'false', // <-- invalid default prop (detected by Flow)
}: Props): Element<*> => (
  <div>{bar && <p>{foo}</p>}</div>
)

const test2 = <MyComponent2 foo="test" />

above snippet in flow repl

EDIT:
So I might have figured it out, but still wondering if Flow should infer this automatically. Using existential type for the destructured props arg, it'll catch the invalid default prop too :)

// @flow
import React from 'react'
import type { Element } from 'react'

// generic type signature for functional React component
type ComponentFn<P: {}> = (props: P) => Element<*>

// define props
type Props = {
  foo: string,
  bar?: boolean,
}

// using function type
const MyComponent1: ComponentFn<Props> = ({
  foo,
  bar = 'false', // <-- invalid default prop (now Flow catches it!)
}: *) => (
  <div>{bar && <p>{foo}</p>}</div>
)

const test1 = <MyComponent1 foo="test" />

How about this?

function foo({surname string, birthDate Date})

If a code..

function foo(dto: {surname: string, birthDate: Date})

.. need to be improved to use destructured object, instead of this proposed syntax..

function foo({(surname: string), (birthDate: Date)})

.. which entails going to each variable and surrounding each of them with parenthesis, it's tiresome and makes the code looks so busy. It would be easier to just remove the colon:

function foo({surname string, birthDate Date})

And when it's need to be renamed, just use the already established syntax for renaming:

function foo({surname string: apellido, birthDate Date: fecha})

Maybe ES6 committee decided that renaming destructured object would be the norm, so they choose a terse syntax to rename something. But why ES6 didn't dream of JavaScript of having types someday, or having TypeScript being part of the browser? Had they thought of it, they would not choose colon for renaming variable, they would use as for renaming, which is symmetrical with import statement. Another alternative would be is not to use an operator at all, same with SQL:

SELECT * FROM Person as p

Not using as would be just as fine:

SELECT * FROM Person p

So renaming surname to apellido would be:

function foo({surname apellido, birthDate fecha})

const {surname apellido, birthDate fecha} = person;

And then when in the not-so-distant future that JavaScript would have types. Boom! they could just use the established syntax for declaring types:

function foo({surname apellido: string, birthDate fecha: Date})

const {surname apellido: string, birthDate fecha: Date} = person;

function foo({surname: string, birthDate: Date})

const {surname: string, birthDate: Date} = person;

Too late.

Will just wait for WebAssembly to take off, and wait for a language that has a nicer syntax :) It might also be our chance for a language to have a decimal type.

I highly suggest this syntax..

function foo({surname string, birthDate Date})

..it's not really weird to remove an operator for declaring types, e.g.

CREATE TABLE Person (
    PersonID  int,
    LastName  varchar(255),
    FirstName varchar(255),
    Address   varchar(255),
    City      varchar(255) 
);

Another advantage of the above over this one..

function foo({surname as string, birthDate as Date})

..is if ECMAScript committee decided to make the renaming use the as keyword so there's symmetry with import's as, this won't cause breaking change:

function foo({surname string as apellido, birthDate Date as fecha})

function foo({surname as apellido, birthDate as fecha})

const {surname string as apellido, birthDate Date as fecha} = person;

const {surname as apellido, birthDate as fecha} = person;

Whereas this one would cause breaking change when as would be introduced for renaming destructured object. Both surname and birthDate would have any type when as would be used for renaming in the future:

function foo({surname as string, birthDate as Date})

const {surname as string, birthDate as Date} = person;

Nearing a year out and we still haven't heard anything about the mentioned coordination with Typescript @jeffmo - and it seems the parentheses syntax last suggested by @akomm but by others too has a plurality of consensus. It seems unlikely we'll be able to find a syntax that meshes perfectly with native syntax, looks nice, and won't conflict with anything else - the parentheses version is probably the closest we'll get. Can we call it good enough and get going on implementing it?

Why should this be any different from the rest of Flow? To me, given how flow works with ':', the following is natural (and what I tried on a guess):

const FunctionalComponent = ({prop1, prop2, ...rest} : {boolean, string, Object}) => (
  <div>
    {doSomethingWith(prop1, prop2)}
    <ChildComp {...rest} />
  </div>
)

This seems the cleanest and most likely lest hacky to implement option, IMNSHO.

@Panoplos This would clash with the existing (working) annotation syntax:

const FunctionalComponent = ({ prop1, prop2, ...rest }: { prop1: boolean, prop2: string }) => (
  <div>
    {doSomethingWith(prop1, prop2)}
    <ChildComp {...rest} />
  </div>
)

Why are the second prop names necessary?

Because it's inline type definition of object that destructuring

It's similar to:

type PropsType = {
  prop1: boolean,
  prop2: string,
}

const FunctionalComponent = ({ prop1, prop2, ...rest }: PropsType) => (
  <div>
    {doSomethingWith(prop1, prop2)}
    <ChildComp {...rest} />
  </div>
)

I can live with this version. Thanks for the pointer! ;^)

@wKich That work around doesn't always work:

//@flow

type input = {
  a?: number,
  b?: string,
  c?: null|number // this causes an error
};

function takeOptions ({ a = 1, b = 'abc', c = 1 }: input = {}) {
  console.log(a);
  console.log(b);
}

takeOptions({ a: 2 });
takeOptions();
takeOptions({});

// this should be an error
//takeOptions({ a: 'string' });

https://flow.org/try/#0PTACDMBsHsHcCh4BcCeAHApgAgJYDs0BXJLAXiwG94ssBDAfgC4s9CBbAIwwCcAaarByZYAzkm74A5vxoBjYa0iQAPq048sILEgAWOEVlm1CIjAdp4sPbtG7wAvgG5E4QnllIc0S0loBrDAB5NE9vAwAKCjoyLABGXkEYgHJaDlkkhNkY2Kx7ZnwiEnIKewBKSgFZMOhIDAA6GElw2lLnOWrahugmjlaHRF8A4NC8EUi6ZgAmXL7BoJCvUfDZ-3mRsZK++C1dfVEdaEJIABNBbAsrbhs7EDnhxY2JrCSxCTxJJJnHIA

Alright, I can see a little stagnation here (or some issue with decision making), so I'd like to sum it up (again?) and move it forward.

Some proposals of syntax evolved during this two and a half year long (!) discussion. All of them vary in details only, some extra braces or colons appear here and there, but general concept stays the same, as far as I see.

In some cases there was no example presented for property alias nor for default value, so I extrapolated the idea by myself. I hope original inventors will forgive me this boldness.

Proposal A ❤️

function f({ a as string, b as number }) {}
// extrapolating idea to aliases and default values
function f({ a as string: aAlias, b as number: bAlias }) {}
function f({ a as string: aAlias = 'a', b as number: bAlias = 1 }) {}

It's possible I mismatched original idea so there are two more variations of it:

A.2

function f({ a: string, b: number }) {}
// extrapolating idea to aliases and default values
function f({ a as aAlias: string, b as bAlias: number }) {}
function f({ a as aAlias: string = 'a', b as bAlias: number = 1 }) {}

A.3

function f({ a: string, b: number }) {}
// extrapolating idea to aliases and default values
function f({ a: string as aAlias, b: number as bAlias }) {}
function f({ a: string as aAlias = 'a', b: number as bAlias = 1 }) {}

Proposal B 👍

function f({ (a: string), (b: number) }) {}
function f({ (a: string): aAlias, (b: number): bAlias }) {}
// extrapolating...
function f({ (a: string): aAlias = 'a', (b: number): bAlias = 1 }) {}

Proposal C 🎉

function f({ a string, b number }) { }
function f({ a string: aAlias, b number: bAlias }) { }
// extrapolating...
function f({ a string: aAlias = 'a', b number: bAlias = 1 }) { }

Proposal D 😄

function f({ a:: string, b:: number })
function f({ a:: string: aAlias, b:: number: bAlias })
// extrapolating ...
function f({ a:: string: aAlias = 'a', b:: number: bAlias = 1 })

As far as I see nobody found serious issues with any of these proposals, it seems to be a matter of opinion and consistency with current syntax. Maybe it's a bias, but it seems idea B got the most support from the community so far.

@mroch @gabelevi @samwgoldman, what do you think about this?

Edit: I've added reaction symbols to each proposal. Maybe it's a bit silly, but this way we could verify community support for each one. Instead of writing comments that are hard to count ;)

Proposal B fits best into the current JS style

C is cleanest...

@Panoplos but inconsistent with regular variable typing in the use of :

Proposal B - consistent use of colon and parenthesis:
const a: string = b vs const {a: string} = b vs const {(a: string): c} = b

Proposal C - inconsistent use of colon:
const a: string = b vs const {a string} = b vs const {a string: c} = b

I dunno... B seems cluttery to me. I would just as rather stick with the existing mechanism than adopt something like B. I am just one fish in this vast ocean, though...

Proposal B is consistent with existing syntax around declaring variable types, casting, etc. I like it and I think it's the most intuitive. It can be verbose in functions with lots of params but that's avoidable by wrapping params on new lines or just falling back to the status quo of using a named object.

Proposal A is easiest to read for me.

a as string: aAlias from A is confusing. ES6 module imports use as to alias, so this suggests to me that a is being aliased to string which has a type of aAlias. With basic types, this less of a concern, but what about when you have { factory as WidgetFactory: widgetFactory }?

B seems the most intuitive. It looks like an existing cast, doesn't introduce new keywords or a new syntactic relationship between variables and types, and is easily explained: if you need to alias a typed property of an object, wrap property: type in parens.

@jordansexton You're right, I didn't understand original idea I suppose. Added two more variants of it.

@erykpiast Nice addition. And I'm not sure I understand the original idea either 😄

A.3 looks the best. {a: string as alias = 'default}

Yes, A.3 is the best, now. Very intuitive.

A.3 (or A.2) is clashing with current syntax, so that's probably a no-go.

A.1 and D use a different token for :. C uses only position. All widely inconsistent.

B is consistent, but onerous. As multiple people mentioned, if that was the syntax, they might prefer not to use it all:

const Quote = ({
  (message: string),
  (quote: quote),
  (name: string),
  (photoUrl: string),
  (photoAlt: string),
}): ReactElement => (
  <div>
    <span>{message}</span>
    <div>
      <q>{quote}</q>
      <div>— <cite>{name}</cite></div>
    </div>
    <img src={photoUrl} alt={photoAlt} />
  </div>
)

Going from f(a: A, b: B) to f({a, b}) (f({(a: A), (b: B)})) is not straightforward.

But no one came up with other intuitive syntax, so it's probably either add B or leave as is.

somebody should make the decision now I think. almost 3 years.

Yes, it's time for this.

Useless bump vote comment for moving forward with B...

@mroch @gabelevi @samwgoldman, I'm just trying to get your attention :)

how about an inline-comment based solution? I remember having a conflict between eslint and flow and using a /* */ comment made both happy

@mroch @gabelevi @samwgoldman we're stuck apparently :( I'd like to know what do you require from community to move it forward.

This should be referenced in a milestone.

This issue thread is too long to comprehend/read for a newcomer to the discussion and that needs to be addressed. Please edit the opening comment by @kittens to include the latest breakdown of the discussion that @erykpiast provided us recently or at least reference it in there.

I prefer propose C:tada:
ref: https://github.com/facebook/flow/issues/235#issuecomment-343083579

FWIW This is my biggest obstacle to using Flow. Destructuring is too powerful and expressive to sacrifice or cripple for static typing. Proposal B please.

I also vote for variant B.
It similar would work on object literals.
Also it's very not comfortable to define outside of class the methods definitions where uses destructuring.

Variant B. It's consistent with current syntax.
And do you have some ETA? The issue was created more then three years ago...

I also vote for B. It's great with current syntax.

please consider adding b. i prefer it to the choice that typescript went with, which is to write the destructured structure twice, once for the matches, and the other for the type names

Happy belated birthday #235! Sorry we missed it... 3 years is a great age, you should stop drooling soon. cc @lili21

B is noisy and error - prone.
D have no benefits over A and C.
So my vote is either A or C. Either of them is ok and much better than current situation.

@dmitry-lexical in what way is B error-prone?

At this point I'd take _any_ proposal as long as it makes this move forward...

It would be cool if the Flow team could provide us with a roadmap of what they are working on, and to see the priority of issues like this.

I don't think I've heard of anyone switching from TS to Flow, but I lot the other way around.

Hi, proposal B is the most likely one the Flow team would support, but it continues to be very low on our priority list. Happy to take PRs.

Hey @avikchaudhuri thanks for taking the time to reply! I was wondering if the Flow team would consider using GitHub to discuss progress/plans/etc for Flow. It seems like a lot of it is happening through internal groups at Facebook, and I think it would be great for the Flow community if they could be a part of it.

@gabelevi says:

We've held off on adding inline type annotation for destructuring since there isn't an obvious place in the grammar for object destructuring (var {a: string, b: number} = x will bind x.a to string and x.b to number). Array destructuring doesn't have this problem...

@samwgoldman says:

@gabelevi, maybe I am being dense here, but why is var {a: string, b: number} = x not possible? Shouldn't the context be enough to disambiguate between destructuring-with-type-annotations and object literal syntax?

Oh, nevermind. I forgot about rebinding destructuring.

I had the same question, and I still don't get it. What's the problem with var {a: string, b: number} = x?

Why would it be bad to bind x.a to string and x.b to number? (With this code, that sounds like exactly what I'd be trying to do -- make sure that x.a is a string and x.b is a number.)

@ESRogs var {a: string, b: number} = x is already valid javascript equivalent to var string = x.a; var number = x.b;. (It's a type of destructuring assignment. The logic of its design is easier to get with silly symmetrical examples like var {p: foo, q: bar} = {p: 42, q: true};.)

It should be noted that proposal A may be confusing if the as destructuring patterns Stage 0 ECMAScript proposal is accepted.

What is the current working solution if I don't want a type defined externally to my function?

@Extarys, var f = function({x}: {x: T}) { /* ... */ } is working for me.

I can't really find documentation for it, although https://flow.org/en/docs/types/objects/#toc-exact-object-types and https://flow.org/en/docs/lang/refinements/ imply it.

As somebody late to the world of flow and this party of a thread... what's the likelihood that something along the lines of proposal B from @erykpiast's comment back in Nov 2017 will be integrated in to flow? That makes so much sense and instills DRY principles - the current implementation certainly has me wanting to just use {} or Object when faced with somewhat involved object destructuring.

@thisissami (and others finding this thread, as it's gotten pretty long): the status was summarized by @avikchaudhuri in a comment about 6 months ago:

Hi, proposal B is the most likely one the Flow team would support, but it continues to be very low on our priority list. Happy to take PRs.

So, the likelihood of it happening is up to you! And me, and "us" -- whoever wants to have some fun with the Flow implementation to work up a PR.

It's been nearly three years since the solution was first mentioned and it still doesn't work. How is this not facebook's top priority with flow? I'd been using it for less than a day when I ran into this.

Facebook’s priorities changed in November 2016, as you might remember.

Sent with GitHawk

What does it mean? Is flow no longer meant to be used for js type
declarations?

Śr., 10 paź 2018, 16:02 użytkownik Ermolay Romanov notifications@github.com
napisał:

Facebook’s priorities changed in November 2016, as you might remember.

Sent with GitHawk http://githawk.com


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/facebook/flow/issues/235#issuecomment-428583659, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAtQF7UfIbP3tlOtJpP-jZMRA5mRyUPcks5ujf3wgaJpZM4DXqCr
.

What does it mean? Is flow no longer meant to be used for js type declarations?

That was a joke, it's implying that political issues are distracting Facebook from tech as a priority. It's not actually clear if the events of the last few years have made flow less important.

Why is it so hard for everybody to understand that Facebook's main priority is for sure not their open-source software? Why can't just everybody be grateful for what they got and stop complaining all the time? It's so annoying 🙄 If you need this feature so much - fork it and contribute. That's how open-source works.

How does this look? It incorporates current syntax like B, but it doesn't require pairs of parens. Also, the parser can't confuse it with destructuring.

function f({ a: string:, b: number: })
function f({ a: string: aAlias, b: number: bAlias })
// extrapolating ...
function f({ a: string: aAlias = 'a', b: number: bAlias = 1 })

How does this look? It incorporates current syntax like B, but it doesn't require pairs of parens. Also, the parser can't confuse it with destructuring.

function f({ a: string:, b: number: })

@chaimleib Parser CAN be confused, like here:

function f({ a: MyClass; })

Is this destructing or typing?

No, in my syntax, that would be function f({ a: MyClass: }), with a colon, not a semicolon. I've never seen semicolons inside of curly braces like that before, what does it do?

Try Flow gives parse errors on both

function f({ a: MyClass: }) {}

and

function f({ a: MyClass; }) {}

Both syntaxes seem to be available. But between these, I still prefer the one with colons.

@chaimleib AFAICS types for function parameter destructuring already work just fine. if you use the appropriate Flow syntax. This thread is about variable assignment, where there is no solution.

The difference is what you show is only destructuring of an existing object. The problem is creating new variables from an existing object, i.e. type-annotating those new variables.

That said, looking at the original very first comment at the top, it has code that actually works just fine now. The function parameter destructuring of the array parameter can be type-annotated. Here is the variable-creating assignment from that comment.

....

Aaaactually, even destructuring object assignment with new variable creation (rebinding destructuring) works now?

So... what exactly is the issue now? And I'm asking for "I don't like that particular syntax" answers, it's a given that a number of people won't like whatever is chosen.

So... what exactly is the issue now? And I'm asking for "I don't like that particular syntax" answers, it's a given that a number of people won't like whatever is chosen.

@lll000111 Currently, we have to do things like this to get object destructuring with type annotations:

const MyComponent = ({
  aProp,
  someOther,
  zethIs,
  onClick,
  onSubmit
}: {
  aProp: string,
  someOther: number,
  zethIs: Object,
  onClick: (event: Event) => void,
  onSubmit: (event: Event) => void,
}) =>

If we need to rename, add or remove a parameter, we have to modify two different lines to make sure the type annotations and the destructured fields stay in sync. And this line-doubling gets really annoying when you are destructuring lots of fields at once.

We can't combine the destructuring with the annotations to get the effect we want:

const MyComponent = ({
  aProp: string, // actually, would rename aProp to string
  someOther: number,
  zethIs: Object,
  onClick: (event: Event) => void, // invalid syntax
  onSubmit: (event: Event) => void,
}) =>

So we are discussing alternate syntaxes that would let us stop repeating ourselves. My syntax proposal follows a nice summary post of other syntaxes above.

The UX is pretty bad, it's like either flow or ES6..ES2019 😞
Please think of something, o enlightened ones! 🙏

Actually we can't even rename a variable during destructuring, which is not ok at all:

const { SecretString: dbSecret }: { dbSecret: string } = await secretsManager.getSecretValue()

Gives the following error:

property `SecretString` is missing in  object type [1].

I can't believe this issue has been opened more than 4 years ago and is still not addressed as it should.

@SherloxFR You can rename a variable during destructuring. That’s the primary reason why this issue hasn’t yet been addressed: to avoid conflicting with that syntax. In your example, you need to annotate the original object that you get from secretsManager.getSecretValue(), not the destructured and renamed variable:

const { SecretString: dbSecret }: { SecretString: string } = await secretsManager.getSecretValue()

one could utilize cast-through-any here (unsafe!)

    for (const [,{name, value}]/*:[string,{name:string,value:mixed}]*/ of (Object.entries(data)/*:any*/)) {
        const _name = name.replace(/\[]$/, '');

see also: https://stackoverflow.com/a/45068255

or, as in my case above, you could realize:
1) if you aren't using both, use Object.keys or Object.values instead.
2) if you are walking an object's keys, you can only expect each value type to be predictable if the object is a Map (ie. { [string]: number }. Otherwise, we must expect the value type to be different in each item of the resulting array! Which means we must handle it at runtime with validation, casting, or switching:

    for (const item of Object.values(data)) {
        const name = (''+ _.get(item, 'name')).replace(/\[]$/, '');
Was this page helpful?
0 / 5 - 0 ratings

Related issues

philikon picture philikon  ·  3Comments

john-gold picture john-gold  ·  3Comments

damncabbage picture damncabbage  ·  3Comments

funtaps picture funtaps  ·  3Comments

cubika picture cubika  ·  3Comments