Skip to content

Flow: property $refType is missing in object #2394

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

Closed
mrtnzlml opened this issue Apr 3, 2018 · 15 comments
Closed

Flow: property $refType is missing in object #2394

mrtnzlml opened this issue Apr 3, 2018 · 15 comments

Comments

@mrtnzlml
Copy link
Contributor

mrtnzlml commented Apr 3, 2018

Hello. 👋 We use generated Flow types in our functions like this for example:

export default (room: ?BeddingInfo_room): string => {
  // ...
};

Problem is that Relay is generating Flow types with +$refType:

import type { ConcreteFragment } from 'relay-runtime';
import type { FragmentReference } from 'relay-runtime';
declare export opaque type BeddingInfo_room$ref: FragmentReference;
export type BeddingInfo_room = {|
  +type: ?string,
  +maxPersons: ?number,
  +bedding: ?$ReadOnlyArray<?{|
    +type: ?string,
    +amount: ?number,
  |}>,
  +$refType: BeddingInfo_room$ref,   // <<<
|};

This, unfortunately, complicates our testing because we cannot use simple plain objects (without the $refType):

expect(
  // Flow error here (vv) because Relay $refType is missing in the object
  formatBeddingInfo({
    type: 'Single Room',
    maxPersons: 1,
    bedding: [
      {
        type: 'Single Bed(s)',
        amount: 1,
      },
    ],
  }),
).toMatchSnapshot();

I don't really understand why is this property necessary but is it possible to make it at least optional? Or how should we test it properly? It was fine before version 1.5... Error message:

Cannot call formatBeddingInfo with object literal bound to room because property $refType is missing in object
literal [1] but exists in BeddingInfo_room [2].

     app/hotels/src/singleHotel/roomList/__tests__/formatBeddingInfo.test.js
      4│
      5│ it('formats bedding information', () => {
      6│   expect(
 [1]  7│     formatBeddingInfo({
      8│       type: 'Single Room',
      9│       maxPersons: 1,
     10│       bedding: [
     11│         {
     12│           type: 'Single Bed(s)',
     13│           amount: 1,
     14│         },
     15│       ],
     16│     }),
     17│   ).toMatchSnapshot();
     18│ });
     19│

     app/hotels/src/singleHotel/roomList/formatBeddingInfo.js
 [2]  7│ export default (room: ?BeddingInfo_room): string => {
@sibelius
Copy link
Contributor

sibelius commented Apr 3, 2018

@mrtnzlml
Copy link
Contributor Author

mrtnzlml commented Apr 3, 2018

@sibelius Oh, that would probably solve my problem. But is it exported outside of the react-relay package? I am not able to find it (or figure out how to use it correctly).

EDIT: I am not sure about that. That won't solve my issue since the $refType is still required (?).

mrtnzlml added a commit to kiwicom/mobile that referenced this issue Apr 3, 2018
I am still not sure how to fix the `$refType` issue. See: facebook/relay#2394
@juhaelee
Copy link

juhaelee commented Apr 8, 2018

@mrtnzlml did you find a solution for this? I believe this issue is related as well: #2316

@mrtnzlml
Copy link
Contributor Author

mrtnzlml commented Apr 8, 2018

@juhaelee I did not. We are just ignoring the errors for now until we find a better solution. I was quite surprised that no one noticed this change so 1) no one upgraded or 2) we are testing it wrong... :)

@juhaelee
Copy link

juhaelee commented Apr 8, 2018

@mrmlnc I see, I also seemed to get flow errors on mutations as well ever since the upgrade :/

@mrmlnc
Copy link
Contributor

mrmlnc commented Apr 8, 2018

@juhaelee, I think you wanted to mention @mrtnzlml :)

@kassens
Copy link
Member

kassens commented Apr 8, 2018

Hey, sorry for not responding here earlier.

To unblock your tests for now, you can do something like:

// Maybe define this in some utility
const mockRefType: any = null;

expect(
  // Flow error here (vv) because Relay $refType is missing in the object
  formatBeddingInfo({
    $refType: mockRefType,
    type: 'Single Room',
    maxPersons: 1,
    bedding: [
      {
        type: 'Single Bed(s)',
        amount: 1,
      },
    ],
  }),
).toMatchSnapshot();

I do think there should be more work done on testability and making the testing more ergonomic.

@roballsopp
Copy link

My theory for the root cause is the flow type for $RelayProps (and therefore createFragmentContainer) is incorrect: #2443. If I'm right, the fix might be pretty simple.

@bastoche
Copy link

bastoche commented Aug 5, 2018

There is actually a workaround described in the relay doc:

Applied to a fragment definition, @relay(mask: false) changes the generated Flow types to be better usable when the fragment is included with the same directive. The Flow types will no longer be exact objects and no longer contain internal marker fields.

To be clear, you need to add @relay(mask: false) both when defining the fragment:

fragment Todo_todo on Todo @relay(mask: false)

and when using it:

...Todo_todo @relay(mask: false)

You will find a working example in this repository.

@sibelius
Copy link
Contributor

sibelius commented Aug 5, 2018

this PR could have solved this #2293

try on master

tbergquist-godaddy pushed a commit to kiwicom/mobile that referenced this issue Jan 24, 2019
I am still not sure how to fix the `$refType` issue. See: facebook/relay#2394
@creatyvtype
Copy link

creatyvtype commented Feb 8, 2019

@sibelius it did not. Just upgraded from 1.4.1 to 2.0.0 and we have hundreds of these flow errors now. the mockRefType workaround that @kassens suggested works, but I can't just go through each error and add this one by one. This is a big problem for us. The same goes for $fragmentRefs

@steida
Copy link

steida commented Feb 8, 2019

Before I switched to TypeScript, I was manually fixing generated types as a workaround.

@artola
Copy link
Contributor

artola commented Feb 10, 2019

@steida By the way, then how did you solved it in TS?

I have a generic (NoFragmentRefs<T>) to cleanup recursively the references and I use it for component data and to prepare the fixtures.

@steida
Copy link

steida commented Feb 10, 2019

@artola Check here https://github.com/este/este

@kassens
Copy link
Member

kassens commented Mar 14, 2019

Check out https://github.com/relayjs/relay-examples/tree/master/todo, @Stephen2 has been working with me to figure out the typing in OSS. The types exported from flow-typed seem to be a bit off and he's been looking in fixing them.

I'm pretty sure they work as defined in relay iself. We use them all over the place internally.

@kassens kassens closed this as completed Mar 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants