Flow: $Shape seems to be broken

[philiiiiiipp]

Flow version: 95.1

Expected behavior

type A = $Shape<{| key: string |}>;

const a: A = {}

a.key.charAt(0) // <- Should be an error

Actual behavior

No errors!

I am actually fairly certain that even though going back some versions in the try environment never produces an error, that this worked at one point.

• Link to Try-Flow or Github repo:

https://flow.org/try/#0C4TwDgpgBAglC8UAkBlAFgQ0gHgN4B8oBrCEALigGdgAnASwDsBzKfAXwD4BuAKB4GMA9g2pQMFOIlxs+GAHQkQc-phoxgACgAMASh5A

[philiiiiiipp]

Maybe it is related to #7526

[noppa]

I don't think this is a bug, this is how $Shape has always worked.
It's an unsound escape hatch that doesn't check missing properties, akin to any.

See #5702, #7316, comment.

A safe alternative to $Shape would be something like

type Partial<T> = {...{...T}}

Try.

Edit 2019-08-29:

type Partial<T> = $Rest<T, {}>

works better now, see comments below

[philiiiiiipp]

What do you mean with missing properties? Do you mean that none of the properties that come from within $Shape will get checked? But why have it then in the first place if it just sets the type to any?

I am _somewhat_ certain that my usecase worked at one point because I am doing my update validation based on the $Shape of the input values and I like to believe that I tested this before using.

[noppa]

No, it doesn't set the type to any, but it lets you use type $Shape<T> as if it was T.
Try.

You can set the Flow version from the top right corner of Flow playground. That example doesn't give errors in any of the older versions either.

$Shape is not safe, but it's better than any. These statements still give an error in your original example:

a.keyyy.charAt(0)
a.key = 5
a.key.toFixed(2)

[philiiiiiipp]

I see, that was totally unclear to me, thanks a bunch! I will change it to the Partial everywhere.

A bit off topic, but you do not happen to know a way to _revert_ $Shape ?

E.g.:

type A = {| optional?: string, nonOptional: string  |}

const a: $MakeEverythingNonOptional<A> = { nonOptional: 'Hallo' } // <- should now give an error

[noppa]

I'm afraid I don't.
There seems to be an issue #5134 about that, though. And look, there's that pesky $Shape again :sweat_smile:

[philiiiiiipp]

Haha ok, thank you very much for your help! I believe you prevented a couple of bugs down the line for sure :-)

[Hypnosphi]

https://github.com/facebook/flow/issues/7566#issuecomment-473882893 apparently stopped working in Flow v0.106, probably because of https://medium.com/flow-type/coming-soon-changes-to-object-spreads-73204aef84e1

[noppa]

@Hypnosphi $Rest<T, {}> works (source: https://github.com/facebook/flow/issues/7730#issuecomment-492985898).

...except for exact type & empty objects -combo, where Flow seems to infer that the type of the empty object is inexact.

1. inexact type ✔️ works
2. exact type with non-empty object ✔️ works
3. exact type with empty object ❌ doesn't work

[goodmind]

@noppa that's just unsealed object?

[noppa]

@goodmind Yeah but shouldn't you be able to assign an empty object (sealed or not) to a type whose properties are all optional? This is probably getting out of scope of this issue, but

type Foo = {|a?: string, b?: string|}

const foo1: Foo = {a: ''} // works as expected
const foo2: Foo = {}      // doesn't work, but should, right?

[philiiiiiipp]

I think ‘{}’ is somehow treated differently. As an object that you can still fill, if you freeze it it should work.

[lyleunderwood]

yeah, unsealed objects are a special case and a common point of confusion. check the section on unsealed objects in the docs. if you change your example to not use an unsealed object, it works as expected:

type Foo = {|a?: string, b?: string|}

const foo1: Foo = {a: ''}
const foo3: Foo = {...null} // this is equivalent to {} but is not an unsealed object

i'm not a fan of unsealed objects in general. i think they're too surprising, it seems like they are the cause of problems more often than they are the solution.

[jedwards1211]

The workaround {...{...T}} is f'ed up. It makes no sense that any amount of spreading a required property should make that property optional.

[noppa]

@jedwards1211 Yeah, which is why it doesn't work anymore. The answer is $Rest<T, {}> now.

From the docs:

For example, $Rest<{|n: number|}, {}> will result in {|n?: number|} because an in-exact empty object may have an n property