Exact fails when doing es6 object spread
MarcoPolo opened this issue · 67 comments
This fails:
// @flow
type A = {|
foo: number
|}
const foo: A = {foo: 3}
const bar: A = {...foo}With error:
demo.js:8
8: const bar: A = {...foo}
^^^^^^^^ object literal. Inexact type is incompatible with exact type
8: const bar: A = {...foo}
^ exact type: object type
This pattern is pretty common in redux style reducers where you return the old state merged with new values (e.g.{...foo, foo: 4})
Yeah, I think your expected behavior is reasonable here. There are quite a few issues surrounding object spread (which is why it has its own issues tag), so we should probably take another look at the entire spread implementation with this and other issues in mind.
Thanks for the report with simple repro code!
@samwgoldman I'm curious if I should expect any activity on this issue and what a good workaround in the meantime might be.
I discovered the same problem and noticed the same error applies when using Object.assign
type Foo = {
a: string,
b: string
};
const baseFoo: Foo = {
a: 'a',
b: 'b'
};
const extendedFoo: Foo = Object.assign(
{},
baseFoo,
{ a: 'c' }
);@samwgoldman I'm curious if I should expect any activity on this issue and what a good workaround in the meantime might be.
@pbomb, the work around I'm using is to not start making use of exact types until spread support improves. I rely heavily on object rest/spread so I can write pure functional JavaScript.
OTOH, exact types seem like more of a nice to have feature. For example, there's a good chance my app will work after doing a large refactor once Flow's happy, but the tests may still fail if I have some extra properties left round that would cause any deep object comparisons to fail. It would be great if Flow caught that stuff too, but it's not the end of the world if it doesn't.
At least that's how it's working out for me anyway...
@pbomb A workaround we use:
type Foo_ = {
a: string,
b: string
};
type Foo = Foo_ & $Shape<Foo_>;
The type $Shape<Foo_> represents the objects which have a subset of the keys {a, b}. An object of type Foo cannot contain other fields than a and b, and the spread mechanism (seems to) works.
@guicar 's solution works perfect for me!
I now use type Exact<T> = T & $Shape<T>; everywhere and it works :o
is there currently any activity on resolving this issue?
As for custom Exact type, it breaks autocomplete in Atom. Just saying.
This is quite a missing key feature when using Redux where you probably use spread operator everywhere in your reducers. If you define an exact state type, flow will report errors when using the spread operator (Inexact type is incompatible with exact type). If you do not define exact type for your state, then you're app state is not safe anymore. Example:
type TAppState = {|
auth: TLoginState;
...
|}
type TLoginState = {|
isLoading: string;
token: string;
|}
...
// Login reducer:
...
return { ...state, isLoading: true }
// Flow error: object literal. Inexact type is incompatible with exact type
Just echoing @ferrannp. Ran into this in the same exact context (Redux reducers).
Specifically, I have a function (reducer) that accepts a param of an exact object type (let's call it SomeType). SomeType has two attributes, each of type AnotherType. I've annotated the function to return SomeType.
I'm attempting to do something like the following:
const (anotherType: AnotherType) = { k: v };
// Copy a `SomeType` with one of the two `AnotherType` fields changed.
return ({
...(someType: SomeType),
attrTwo: anotherType,
}: SomeType)I have found this workaround to work quite well for our redux reducers:
export type Foo = {
a: string,
b: string,
}
type Exact<T> = T & $Shape<T>;
function reducer(state: Foo = {}, action: any): Exact<Foo>{
return {
...state,
a: 'hello',
}
}Using the Exact<Foo> as returning type of the reducer allows us to use the spread operator. And then by referring to Foo elsewhere in the application gives use autocompletion of properties.
Thanks to @guicar for providing the Exact method.
Is this indeed considered a bug? An entry in the docs right around "Exact objects" would be helpful!
@guicar and @torarnek your workarounds do not include an exact object type, and so don't address this isssue.
No errors:
type Exact<T> = T & $Shape<T>;
type Foo = {
a: string,
b: string
};
const reducer = (state: Foo): Exact<Foo> => ({
...state
});
Now make Foo exact:
type Exact<T> = T & $Shape<T>;
type Foo = {|
a: string,
b: string
|};
const reducer = (state: Foo): Exact<Foo> => ({
...state
});
Error: object literal. Inexact type is incompatible with exact type.
I am still struggling to find a way to clone an exact-typed object, without writing out every single one of its top level properties.
@samwgoldman let us know if there is anything we can do to help on this... : )
Really missing this in redux, where you can misspell a state key.
Agreed-- I would really like to defined my Redux state object as exact and be able to use the spread operator in my reducer, though Flow does not like that. I have not seen any reasonable work-arounds for this yet.
Does this work in TypeScript?
To my knowledge, exact types are not expressable in typescript yet.
type Exact<T> = T & $Shape<T>;
this kind of exact type solve the problem with spread opertor. but it also create a new problem. this is a workground:
type Obj = {
a: {
b: number
}
};
type ExactObj = Exact<Obj>;
const A: ExactObj = {
a: {
b: 1
}
};
let {
a: {
c
}
} = A;in this workground, no error will happen though c is not a key of type Obj and i think this new problem brings more hramness than can't use spread operator with exact type.
Probably that issue need to be pinned to the top
@torarnek great solution! Did you incur into any problems with that?
Oh yes. It doesn't work with covariant (read-only) properties.
May I ask why this issue was closed? I would like to use exact type together with covariant properties in Redux. My current solution is to drop exact types, but I'm not happy about it.
May I ask why this issue was closed?
...it isn't closed, and hasn't ever been closed?
I think most of the descriptions in this issue slightly overstate it.
It seems to be very specifically that trying to pass the result of spreading an inexact and exact type to something expecting an exact type that will trigger an error.
// @flow
type A = {| a: string |};
type B = { b: string };
declare var a: A;
declare var b: B;
const c = { ...a };
const d = { ...b };
const e = { ...a, ...b };
const f: { a: string, b: string } = { ...a, ...b };
// Only this case triggers an error:
const g: {| a: string, b: string |} = { ...a, ...b };However, I have also identified a couple other (unrelated?) behaviours that I believe to be bugs:
// @flow
type A = {| a: string |};
type B = { b: string };
declare var a: { ...A, ...B };
// triggers flow error
// Why has b been inferred to be an optional property?
const x: string = a.b;// @flow
type A = {| a: string, [string]: any |};
// triggers flow error
// I'm not sure what defined behaviour should be here, but
// it's definitely not documented yet:
const x: A = {
a: '',
b: '',
};It seems to be very specifically that trying to pass the result of spreading an inexact and exact type to something expecting an exact type that will trigger an error.
That is true 👍
The thing that is broken seems to be spread of two exact object types not being exact anymore:
// @flow
type A = {| a: string |};
type B = { b: string };
type C = {| c: string |};
declare var a: A;
declare var b: B;
declare var c: C;
const d = { ...a };
const e = { ...b };
const f = { ...a, ...b };
const g: { a: string, b: string } = { ...a, ...b };
// This case triggers an error, as it should:
const h: {| a: string, b: string |} = { ...a, ...b };
// This case should not trigger an error, but it does
const i: {| a: string, c: string |} = { ...a, ...c };Ah right. Again, it's not the spreading itself that errors, but inferring that the result is exact that it fails to do.
// @flow
type A = {| a: string |};
type B = { b: string };
type C = {| c: string |};
declare var a: A;
declare var b: B;
declare var c: C;
// Now this case doesn't trigger an error:
const i: { a: string, c: string } = { ...a, ...c };Again, it's not the spreading itself that errors, but inferring that the result is exact that it fails to do.
Yeah, I get what you're saying. Maybe the issue title / description could be clarified to tell this story in a more clear way?
Maybe "ES6 object spread over exact object type incorrectly marks the resulting object as inexact" could be better?
Maybe "ES6 object spread over exact object type incorrectly marks the resulting object as inexact" could be better?
That is definitely bad behaviour.
@samwgoldman Do you have any advice for someone trying to take a stab at this in the code-base? (files to start exploring, etc)
I would also appreciate any documentation on making it easier to contribute to the flow codebase for this and other issues
In my project I have created a tiny helper function that looks like this:
const assign = <T>(a: T, ...b: $Shape<T>[]): T => Object.assign({}, a, ...b)You can see it in action here: Try Flow
@mroch @samwgoldman What can we do to help here? Should we just use exact types? Please advice.
This is the top issue in terms of 👍s: https://github.com/facebook/flow/issues?q=is%3Aissue+is%3Aopen+sort%3Areactions-%2B1-desc
Looking forward to when it will be prioritized :)
This is easily the most frustrating bug preventing us from using exact types and causing uncertainty around Flow.
+1
@lloiser's workaround isn't workable with covariant properties, right? https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoALgTwA4FMwDK6AhuvgLxgDeAPqmGANRRxwBcYAzugE4CWAOwDmqGgF9UqKAFcBAY3R84Arujg9cACm6lcHIroCU+kmWr0wG9NJ4qA8gCMAVrgUA6Yp058hAzVTEAGlVdYKoWdjAAIgdiAC8osUNUCUlgYDAAQS8fFQALXBg8Hi5pISFcblwAEzBBMDz0dGxONnShPnQ86Qc3OTgAW2AoYjlcB1YAa2H4BGA+L2lK4AAmABYABgBWAGIFziX+gYHcAXQAWgBmNYAOAEZLgHY7u63UfoFuME9vXzBKAA8ABUAHyaYgcIHBNwwhwcAAkBDyxDwwJBAG0ALrGMBA-4gsCOFzuH65fxBb7Q2HJGTyRTKVTqXAAfQAbittKY9IQuTiDGYqBYrDYVKTfJzQtQIhwYvFEskxEA
This workaround should work for all of you.
function reduce<T: {}>(object: T, ...rest: Array<$Shape<T>>): T {
return Object.assign((({}: any): T), object, ...rest);
}
const foo: {|a: number, b: number|} = {a: 0, b: 1};
const bar: {|a: number, b: number|} = reduce(foo, {b: 20}, {a: 10}); // no errors
console.log(bar); // {a: 10, b: 20}@joshuawu still no luck with covariant properties though :(
https://flow.org/try/#0GYVwdgxgLglg9mABAJwKYBMQVQHgCoBciA3gL4B8AFHAEYBWq0ReANIgHSdoDOURAgsmQBDAJ44AJAGUAFsIAOuPOXIBKZiQBQiFKighkSAPL1GUdsO7cYAczCUHZIsLCj1iPKra0G0Np3YeKFUAbk1STU0IBF5EYDg4ImIAHwBqYSIwEABbGlRkNlSaTJy85GTSRABeEgzEAAY2YsQARlIw6LBYmmFkJLS6rNz8wuahsorq3UxsSni4NmJmgCZ60kW6lrXQnQB6XcQwOER85DhkbiiYuAAbVHYbuBtKHuQdxH3aoi2molXSIA
function reduce<T: {}>(object: T, ...rest: Array<$Shape<T>>): T {
return Object.assign((({}: any): T), object, ...rest);
}
const foo: {|+a: number, +b: number|} = {a: 0, b: 1};
const bar: {|+a: number, +b: number|} = reduce(foo, {b: 20}, {a: 10}); // Covariant property `a` incompatible with contravariant use
console.log(bar); // {a: 10, b: 20}@joshuawu I guess none of us here want to replace the object spread syntax with object.assign.
We shouldn't have to change our code because of a bug in Flow's typechecking.
Given that both Redux and Flow are Facebook projects and that this code pattern is so common, I'm surprised to see this hasn't been solved yet. It's been about 1.5 years since the issue was opened and while I have no choice but to continue using Flow for the project I'm working on, I can understand why people are moving away from Flow to other static type solutions.
@denizdogan redux is not facebook's project. In fact from what I know it's not even used inside facebook (at least not at scale).
@Andarist My bad, I just assumed it was FB because it's reactjs/redux!
@denizdogan reactjs is community org
I'm facing the same problem exporting actions like written below (with spreaded imports in actions/index.js).
Do you think it is related to the issue?
Thanks
/src/actions/searchActions.js
/* @flow */
type SearchChangeActionType = {
+type: string,
name: string,
value?: string,
};
export const searchInputChange =
({ name, value }: SearchInputType): SearchChangeActionType => {
return {
type: actionsTypes.SEARCH_INPUT_CHANGE,
name,
value,
};
};
/src/actions/index
/* @flow */
import * as formActions from './formActions';
import * as searchActions from './searchActions';
export default {
...formActions,
...searchActions,
};In a component
/* @flow */
import actions from '../../actions';
import type { DispatchType } from '../../types';
...
const mapDispatchToProps = (dispatch: DispatchType) => {
return {
onSearchChange: (value: string) => {
dispatch(actions.searchChange({ // ---------> NO ERROR if i replace the arg by a number
name: 'users',
value: 'Tony',
unWanted: 'foo', //----------------------> NO ERROR
}));
}
};
};Edit
Switching from
import actions from '../../actions';
...
dispatch(actions.searchChange(...));to
import { searchChange } from '../../actions/searchChange';
...
dispatch(searchChange(...));makes the flow typing work
but leads to messy imports and to more function names conflicts..
@MaxInMoon Try this
export * from './formActions';
export * from './searchActions';@TrySound thanks for the response. It's works perfecly but it force to import and use actions like so:
import { myAction } from './actions';But for actions I largely prefer:
import actions from './actions';
// actions.myAction()@MaxInMoon And this
import * as actions from './actions';
// actions.myAction()@TrySound 👍 thanks!
Should we consider the use of import * as actions from './actions'; instead of import actions from './actions'; (to preserve actions type checking) as a bug?
If no it could be good to add a note on the doc, no?
It's how you should use imports. Using spread operator with module namespaces is probably not considered case in flow. In any case it's a hack and should be avoided since you have a better tool.
Also, if you spread with an untyped value it disables the exact check.
type Message = {| foo: number |}
function bar(): Message {
const opts = ({foo: 1}: any)
const message = {...opts, bar: 1}
return message
}
// No error.-
barwill always be in the returned object, and it is not allowed in the exact type. An error should be shown. This is a bug. -
At least show me a warning if you are disabling my exact type check. E.g.
`message` may not satisfy exact type `Message` because an `any` object was used in a spread.
@vjpr Can you try running the flow command with the --include-warnings flag? Flow does have warnings for quite a few situations, but most of those warnings are disabled/silenced. For example, that snippet should trigger the unclear-type warning if you add this to your .flowconfig:
[lints]
; Triggers when you use any, Object, or Function as type annotations. These types are unsafe.
unclear-type=warn
I noticed that the trick mentioned here seems to allow for spreading on an exact-ish type as well.
I noticed that the trick mentioned here seems to allow for spreading on an exact-ish type as well.
It works... somewhat. However, we lose the type safety when we spread an inext object type to the "exact-ish type", and then all bets of which extraneous keys there are, is off.
// @flow
// Type A is an "exact-ish object type"
type A = { a: string, [any]: empty };
// Type B is our good ol' inexact object type
type B = { b: string };
// Type C is also an "exact-ish object type"
type C = { c: string, [any]: empty };
declare var a: A;
declare var b: B;
declare var c: C;
const d = { ...a };
const e = { ...b };
const f = { ...a, ...b };
// This case has always worked
const g: { a: string, b: string } = { ...a, ...b };
// This case SHOULD trigger an error -- We can't guarantee that the
// non-exact object `b` does not have other keys besides `b` set
const h: { a: string, b: string, [any]: empty } = { ...a, ...b };
// This case should not trigger an error, as both `a` and `c` have
// defined with the `[any]: empty` hack that they don't have other
// keys besides `a` and `c` set to strings.
const i: { a: string, c: string, [any]: empty } = { ...a, ...c };Another work-around for this (credit to #5551):
type Foo = {| a: number |};
const x: Foo = { a: 2 };
const x1: Foo = {...x}; // error
const x2: Foo = Object.freeze({...x}); // works as expected
const x3: Foo = {...x, a: 23}; // error
const x4: Foo = Object.freeze({...x, a: 23}); // works as expected
Any progress on this issue?
It's happening: 1916b7d!
If you switch version to master, the original example from the issue already shows no errors! 🎉
It's happening: 1916b7d!
Super awesome! Unfortunately that is a bit too lax, though:
// @flow
type A = {| a: string |};
type B = { b: string };
type C = {| c: string |};
declare var a: A;
declare var b: B;
declare var c: C;
var b: B = { b: 'foo', j: 'I am an extra property!' };
const d = { ...a };
const e = { ...b };
const f = { ...a, ...b };
const g: { a: string, b: string } = { ...a, ...b };
// This case SHOULD trigger an error, but it does not!!
const h: {| a: string, b: string |} = { ...a, ...b };
// This case should not trigger an error
const i: {| a: string, c: string |} = { ...a, ...c };It does not work in JSX (
@AlexanderShushunov It should work in 0.71.
357ef4f
This issue was getting out of hand. Might make sense to open new one, if such an issue doesn't already exist.
If someone gets to it before me, it would be nice :)
Ah, yeah, lets open a new one for that 👍
^ opened: #6425