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

Flow types in RelayTypes stripped from npm module #1064

Closed
sgwilym opened this issue Apr 19, 2016 · 9 comments
Closed

Flow types in RelayTypes stripped from npm module #1064

sgwilym opened this issue Apr 19, 2016 · 9 comments

Comments

@sgwilym
Copy link
Contributor

sgwilym commented Apr 19, 2016

This may be a duplicate of #712, but it looks like this isn't a closed case. I'm using Flow more and more in my projects, and would like to be able to use Relay’s already-defined flow types to augment that.

Previously I've seen some maintainers say that all the types are exposed in RelayTypes, but it looks like all the type definitions are stripped from react-relay/lib/RelayTypes.js, so I can't imagine how that would work.

import type { RelayContainer } from 'react-relay/libs/RelayTypes';

Is this the intended way to use Relay’s flow types? And if so, how does that work considering none of the type definitions are available in the published version of Relay?

@wincent
Copy link
Contributor

wincent commented Apr 19, 2016

Yep, using RelayTypes is the intention. If they're getting stripped then I think we need to do something to make them available.

For example, we could ship a .flow file (like this one), although I am not sure how those work in practice. Another possible option is to figure out if the transform-flow-comments Babel plugin can help us here. I know that can turn annotations into comments; I don't know whether we can leverage that to help us replace these stripped export type lines.

Finally, I guess you could work around this by cloning the Relay source and using npm link, at least for local development, so that you can type check against this file directly. Perhaps somebody who has thought about this for longer than me knows whether this was our intention all along?

@bsr203
Copy link

bsr203 commented May 11, 2016

I closed the linked issue out of frustration. sorry.

@wincent does the file
https://github.com/facebook/relay/blob/master/src/tools/RelayTypes.js
exports all the types. I was looking for the type of a connection query, which I think is defined at
https://github.com/facebook/relay/blob/master/src/interface/RelayOSSConnectionInterface.js
But I don't see a type for it in the RelayTypes.js. How can I get the type of a query response. Thanks.

@wincent
Copy link
Contributor

wincent commented May 11, 2016

The intention is that RelayTypes should export the "public" types. ie. the ones that correspond to stable, public APIs in Relay that are less likely to change, and which framework users may find useful in their own annotations.

Compare that with RelayInternalTypes, which is where we put types that are for internal use within the framework itself. Generally these are in here in a shared file because they are relevant to a bunch of different modules and there may not be a logical single module for us to put them in (there are a bunch of other types that do live in specific modules, because that's the logical place for them). We mark the RelayInternalTypes as internal because they are tied to implementation details, and may change as we refactor, so we don't want people to use them and then suffer the consequences of churn.

To your question about query response type, we don't really have one for that, as far as I know. See payload here is not annotated. I think this is probably because it's going to be so generic ({[key: string]: mixed} A.K.A. Object) as to be not super useful.

@nmn
Copy link

nmn commented May 20, 2016

@wincent Since flow prefers .flow files anyway, if you just publish the original source files to npm along with the transpiled files with the .flow extension, flow will just pick the types up anyway.

So if lib/Relay.js is the transpiled file, lib/Relay.js.flow is the original source file.
You don't have to do any extra work. Just change the build process to copy the original files with the added extension and you're done. You may of course want to make exceptions for certain files, where you may want to manually override the type definitions so that it makes more sense for external use.

Would you please consider doing that?

@jamiebuilds
Copy link

A command to help this workflow is being worked on over here facebook/flow#2184

@nmn
Copy link

nmn commented Aug 9, 2016

@thejameskyle That's awesome!

In the mean time, I still think it's a good idea to publish the original source to NPM for the flow types.

@edvinerikson
Copy link
Contributor

It's not as easy as just copying the source files. Relay uses haste as module system and to be able to use the flow types in a commonjs project we will need to do some transpilation.
I spent a day on this a few months ago and almost got it working except that Babel had a bug that outputted invalid flow syntax (there was something in the parser that made it output some stuff twice) so I couldn't get the .flow files valid. The reason there still is a need for transpilation is because we need to re-write all import paths to be valid in commonjs.

@thejameskyle Is this something that the new command will be able to do? (re-write paths or bundle all flow types into one file)

@crismali
Copy link

Are there any plans to add Relay to flow-types?

@wincent
Copy link
Contributor

wincent commented May 27, 2017

Superseded by #1758 (which applies to Relay Modern), so going to close this one.

@wincent wincent closed this as completed May 27, 2017
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

7 participants