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

Exact fails when doing es6 object spread #2405

Closed
MarcoPolo opened this issue Sep 6, 2016 · 67 comments
Closed

Exact fails when doing es6 object spread #2405

MarcoPolo opened this issue Sep 6, 2016 · 67 comments

Comments

@MarcoPolo
Copy link

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})

@samwgoldman
Copy link
Member

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!

@pbomb
Copy link

pbomb commented Sep 27, 2016

@samwgoldman I'm curious if I should expect any activity on this issue and what a good workaround in the meantime might be.

@jfmengels
Copy link

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' }
);

@dchambers
Copy link

@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...

@guicar
Copy link

guicar commented Oct 12, 2016

@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.

@dralletje
Copy link

@guicar 's solution works perfect for me!
I now use type Exact<T> = T & $Shape<T>; everywhere and it works :o

@i-manolov
Copy link

is there currently any activity on resolving this issue?

@steida
Copy link

steida commented Dec 6, 2016

As for custom Exact type, it breaks autocomplete in Atom. Just saying.

@ferrannp
Copy link

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

@dsimmons
Copy link

dsimmons commented Dec 15, 2016

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)

@torarnek
Copy link

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.

@mull
Copy link

mull commented Feb 14, 2017

Is this indeed considered a bug? An entry in the docs right around "Exact objects" would be helpful!

torarnek added a commit to torarnek/flow that referenced this issue Feb 14, 2017
Ref facebook#2405, document limitation and workaround of exact object types with the spread operator.
torarnek added a commit to torarnek/flow that referenced this issue Feb 14, 2017
Ref facebook#2405, document limitation and workaround of exact object types with the spread operator.
@tuff
Copy link

tuff commented May 23, 2017

@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.

@steida
Copy link

steida commented May 23, 2017

@tuff Impossible with Flow 46. Don't use exact types. It's bummer, I know.

@skovhus
Copy link

skovhus commented Jun 16, 2017

@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.

@royrwood
Copy link

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.

@skovhus
Copy link

skovhus commented Jun 29, 2017

Does this work in TypeScript?

@royrwood
Copy link

@skovhus -- I'm not sure if it works in TypeScript. I haven't tried....

@aaronjensen
Copy link

To my knowledge, exact types are not expressable in typescript yet.

@Leooonard
Copy link

Leooonard commented Jul 23, 2017

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.

@MaxInMoon
Copy link

MaxInMoon commented Mar 10, 2018

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..

@TrySound
Copy link
Contributor

@MaxInMoon Try this

export * from './formActions';
export * from './searchActions';

@MaxInMoon
Copy link

@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()

@TrySound
Copy link
Contributor

@MaxInMoon And this

import * as actions from './actions';
// actions.myAction()

@MaxInMoon
Copy link

@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?

@TrySound
Copy link
Contributor

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.

@vjpr
Copy link

vjpr commented Mar 10, 2018

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.
  1. bar will always be in the returned object, and it is not allowed in the exact type. An error should be shown. This is a bug.

  2. 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.

@mandx
Copy link

mandx commented Mar 10, 2018

@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

@vjpr
Copy link

vjpr commented Mar 11, 2018

@mandx Did not show any warnings.

@Ashoat
Copy link

Ashoat commented Mar 23, 2018

I noticed that the trick mentioned here seems to allow for spreading on an exact-ish type as well.

@valscion
Copy link
Contributor

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 };

Try Flow

@cprodescu
Copy link

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

Try

@apolishch
Copy link

Any progress on this issue?

@niieani
Copy link

niieani commented Apr 6, 2018

It's happening: 1916b7d!

dancing

If you switch version to master, the original example from the issue already shows no errors! 🎉

@valscion
Copy link
Contributor

valscion commented Apr 6, 2018

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 };

Try Flow

@AlexanderShushunov
Copy link

It does not work in JSX (

Try

@TrySound
Copy link
Contributor

@AlexanderShushunov It should work in 0.71.
357ef4f

@0x24a537r9
Copy link

@mrkev Should this have been closed when @valscion raised a valid bug in the fix?

@valscion
Copy link
Contributor

valscion commented Jun 7, 2018

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 :)

@mrkev
Copy link
Contributor

mrkev commented Jun 7, 2018

Ah, yeah, lets open a new one for that 👍

@rattrayalex-stripe
Copy link

^ opened: #6425

@dancrumb
Copy link

dancrumb commented Sep 6, 2018

I've opened #6854 to address the fact that this is still a problem with interfaces

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests