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

[Modern:rc.2] Generated fragment flow type info is being lost on import #1689

Closed
nirrek opened this issue Apr 25, 2017 · 7 comments
Closed

Comments

@nirrek
Copy link

nirrek commented Apr 25, 2017

When using an imported flow type for a fragment that was generated by relay-compiler the type information seems to be getting lost. Using flow type-at-pos will report the type as (unknown). This is when using [email protected] and [email protected].

There's a simplified repo here demonstrating the issue: https://github.com/nirrek/relay-modern-flow-types. I've also inlined the relevant code in the issue below as there's not much to it.

// File: schema.graphql
type User { username: String! }
type Query { viewer: User }
// File: index.js
// @flow
import { graphql } from 'react-relay';
import type { index_user } from './__generated__/index_user.graphql.js';
graphql`fragment index_user on User { username }`;
// `flow type-at-pos` will report `user` as being of `(unknown)` type
const user: index_user = {
  username: 3, // Should be a flow error, but isn't
};
// File: __generated__/index_user.graphql.js
/**
 * This file was generated by:
 *   relay-compiler
 *
 * @providesModule index_user.graphql
 * @generated SignedSource<<075214429c05de848f645f90156b41a8>>
 * @flow
 * @nogrep
 */

'use strict';

/*::
import type {ConcreteFragment} from 'relay-runtime';
export type index_user = {
  username: string;
};
*/

/* eslint-disable comma-dangle, quotes */

const fragment /*: ConcreteFragment*/ = {
  "argumentDefinitions": [],
  "kind": "Fragment",
  "metadata": null,
  "name": "index_user",
  "selections": [
    {
      "kind": "ScalarField",
      "alias": null,
      "args": null,
      "name": "username",
      "storageKey": null
    }
  ],
  "type": "User"
};

module.exports = fragment;

I'm not familiar enough with the inner working of flow to figure out what's going wrong here. There are two different ways I found that allow the index_user type to propagate properly though, so maybe that will offer some clues as to what's going on:

  1. Use export default fragment instead of module.exports = fragment
  2. Remove the ConcreteFragement type annotation on fragment.
@kassens
Copy link
Member

kassens commented Apr 25, 2017

Thanks for the easy repro case, I reduced it a bit more and it seems like imported types from untyped modules are different from any.

https://gist.github.com/kassens/70e59033630c3a24af236225bbfc4f21

@nirrek
Copy link
Author

nirrek commented Apr 26, 2017

It looks like if you import a type that is not declared in the module you are importing from, it will have an (unknown) type. If a value with an (unknown) type is assigned to module.exports any other exported type information from that module seems to be lost. If you export default a value with an (unknown) type, however, the other exported type information is not lost.

Is there some intention behind adding type imports/annotations to generated files for types that don't actually exist in the published module?

A fix for anyone else who hits this issue is to add a flow library definition file for relay-runtime to your project in which you declare the types being imported in the various relay-compiler generated files. In my case I have:

// File .flowconfig
[ignore]

[include]

[libs]
./lib

[options]
// File: lib/relay-runtime.js
declare module 'relay-runtime' {
  // Public API values
  declare var Environment: any;
  declare var Network: any;
  declare var RecordSource: any;
  declare var Store: any;

  declare var areEqualSelectors: any;
  declare var createFragmentSpecResolver: any;
  declare var createOperationSelector: any;
  declare var getDataIDsFromObject: any;
  declare var getFragment: any;
  declare var getOperation: any;
  declare var getSelector: any;
  declare var getSelectorList: any;
  declare var getSelectorsFromObject: any;
  declare var getVariablesFromObject: any;
  declare var graphql: any;

  declare var ConnectionHandler: any;
  declare var ViewerHandler: any;

  declare var commitLocalUpdate: any;
  declare var commitMutation: any;
  declare var fetchQuery: any;
  declare var isRelayStaticEnvironment: any;
  declare var requestSubscription: any;

  // Internal types imported in compiler generated files
  declare type ConcreteFragment = any;
  declare type ConcreteBatch = any;
}

@ekosz
Copy link

ekosz commented Apr 27, 2017

If I'm reading this issue correctly, does that mean that the flow typing is not working at all at the moment?

@nirrek
Copy link
Author

nirrek commented Apr 29, 2017

@ekosz Yes and no. The flow type that is generated for the fragment is correct. But there are other flow types that are added to the generated file that do not exist. Flow doesn't appear to be able to deal with these non-existent types in a sensible manner (under certain conditions), and leads to us not being able to import any other types from that module. Therefore, even though the fragment type is correct, when we try to import the fragment type, flow incorrectly thinks it has an (unknown) type. In effect, we now have no type information.

If you look at the original code I pasted, the problem lines are these ones:
import type {ConcreteFragment} from 'relay-runtime';
const fragment /*: ConcreteFragment*/ = {

The ConcreteFragment type does not exist in the relay-runtime package (it exists in the source code, but not in the built package that is distributed on npm).

Flow gets all confused because of this non-existent type. To work around this, we can provide our own library definition file for relay-runtime that declares an actual type for ConcreteFragment. This stops flow from getting confused, and so when we import our fragment type we get its actual type instead of an (unknown) type.

In the library definition file I pasted above, the relevant lines are these ones:

// Internal types imported in compiler generated files
declare type ConcreteFragment = any;
declare type ConcreteBatch = any;

ConcreteFragment is added to files generated for fragments, ConcreteBatch is added to files generated for queries. There is likely different types added to other generated files (eg those generated for mutations) but I haven't played around much yet, so I'm not sure what those are. The rule-of-thumb would just be to look at the generated files and add a declaration to the library definition file for any imported types from relay-runtime as relay-runtime doesn't export any types at all atm.

@nirrek
Copy link
Author

nirrek commented May 21, 2017

See #1758 for a discussion around resolving this issue.

@wincent
Copy link
Contributor

wincent commented May 27, 2017

Closing in favor of #1758 to keep all the discussion in one place.

@bchenSyd
Copy link

bchenSyd commented Jun 6, 2017

@nirrek thanks for sharing!

I also add module.file_ext to my .flowconfig so that the extension can be auto-resolved

[options]
module.file_ext=.graphql.js
module.file_ext=.js
module.file_ext=.jsx
module.file_ext=.json

I wish flow can have a resolve/root config like in webpack and jest , so that I can add __generated__ in and endup with just typing:

import type {
  Todo_todo,
} from './Todo_todo';  // this will be resolved as ./__generated__/Todo_todo.graphql.js by flow

that would be amazing!

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

5 participants