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

babel-preset-react-app: add babel-plugin-relay #2343

Closed
wants to merge 2 commits into from

Conversation

maletor
Copy link

@maletor maletor commented May 23, 2017

Adds support for Relay Modern. relay-compiler, relay-runtime, and
react-relay are all "peer" dependencies, including the script to run
relay compiler. This at least unblocks CRA from being able to develop
Relay Modern.

https://facebook.github.io/relay/docs/babel-plugin-relay.html
https://facebook.github.io/relay/docs/relay-compiler.html


#2001 (comment)

Because Relay Modern does not require a schema for babel, this makes the integration process so much easier. Hopefully we can get this merged, but if you think it would be blocked by needing documentation, let me know what a general idea of what you're looking for and we can get that added as well.

Adds support for Relay Modern. relay-compiler, relay-runtime, and
react-relay are all "peer" dependencies, including the script to run
relay compiler. This at least unblocks CRA from being able to develop
Relay Modern.

https://facebook.github.io/relay/docs/babel-plugin-relay.html
https://facebook.github.io/relay/docs/relay-compiler.html
@gaearon
Copy link
Contributor

gaearon commented May 23, 2017

Hmm. This doesn't look complicated. Any caveats?

@maletor
Copy link
Author

maletor commented May 23, 2017

Caveats.

  1. Does not work with relay classic. For instance, if you need to specify a schema, you are out of luck. In my opinion, the complications are not worth it. Either use Relay Modern, or eject.

  2. Wasted CPU cycles on non Relay projects. We aren't going to get into the business of "detecting" a Relay Modern app. This is not trivial while an extra plugin in babel is trivial.

  3. You must be responsible for add relay-compiler yourself as a devDependency and "watching" for updates to your files. This is not unique to CRA and is outlined in the Relay Modern documentation. This is why I say, maybe some documentation on this side would be nice, perhaps pointing at Relay Modern documentation?

I have tested this by directly adding this change within my node_modules directory and it works great!

@gaearon
Copy link
Contributor

gaearon commented May 24, 2017

Does it break any other graphql solutions, like Apollo?

@gaearon gaearon mentioned this pull request May 25, 2017
@gaearon
Copy link
Contributor

gaearon commented May 26, 2017

Can we make a PR to Babel plugin to enforce import/require name?

@wtgtybhertgeghgtwtg
Copy link
Contributor

It looks like the plugin and runtime are designed to be used verbatim, so I'm not sure if that would be an option.

@arvigeus
Copy link
Contributor

arvigeus commented Jun 5, 2017

What about reading package.json and looking for packages like relay-compiler and react-relay in order to use this plugin? No wasted cycles, no need to worry about Apollo (I think)

@wtgtybhertgeghgtwtg
Copy link
Contributor

I don't see an issue for this on the Relay repo and apparently package detection is non-trivial; is there a plan to move forward with this?

@maletor
Copy link
Author

maletor commented Jun 20, 2017

I hope so. Is CRA doing package detection anywhere else at the moment? I would have preferred to keep this as a one line ish change.

@k15a
Copy link

k15a commented Jun 20, 2017

There is process.env['npm_package_devDependencies_react-relay'] which should make package detection pretty easy. Or I am missing an important thing here? I am not aware of a place where package detection is used but there is a place where a value from package.json is read. This could be used for package detection as well.

@gaearon
Copy link
Contributor

gaearon commented Jun 27, 2017

We need some plan for a bunch of optional dependencies people want: Sass, Relay, TypeScript. These are all in principle possible to support but we don't want to either create a second plugin system (which webpack already "is") nor to increase default download size. If you figure out a creative way out of this problem I'd love to know!

@maletor
Copy link
Author

maletor commented Jun 29, 2017

This should probably be merged without that system. It's pretty innocuous.

@maletor
Copy link
Author

maletor commented Aug 17, 2017

Ping @gaearon

@dpehrson
Copy link

dpehrson commented Sep 19, 2017

I'm not sure whether I've just done something stupid, but attempting to apply the same monkey-patch to my own copy of the babel-preset-react-app doesn't even work. Compilation still fails with graphql: Unexpected invocation at runtime. Either the Babel transform was not set up, or it failed to identify this call site. Make sure it is being used verbatim as graphql.

Would definitely like to see proper relay modern support in CRA, the majority of the reason I went with CRA was because I thought it would save me from all this.

@lpalmes
Copy link
Contributor

lpalmes commented Sep 19, 2017

Hey @dpehrson, i have this react-scripts fork, which if you replace your react-scripts dependency in your package.json will work with relay-modern, i'm using them myself, that's why is up in npm, hopefully they are useful to you, and i do hope as well we have relay support in cra in the future.

@react-scripts-dangerous

Hello! I'm a bot that helps facilitate testing pull requests.

Your pull request (commit 56d72ae) has been released on npm for testing purposes.

npm i [email protected]
# or
yarn add [email protected]
# or
create-react-app [email protected] folder/

Note that the package has not been reviewed or vetted by the maintainers. Only install it at your own risk!

Thanks for your contribution!

@utkez
Copy link

utkez commented Oct 24, 2017

I get the following error after creating a new react app, running yarn start and trying to access the homepage:
./~/react-error-overlay-dangerous/lib/components/code.js Module not found: Can't resolve 'react-dev-utils/ansiHTML' in '/Users/utkez/Projects/servers/drac_worker/new-ui/node_modules/react-error-overlay-dangerous/lib/components'

I don't know if it's somehow related, but it seems that exactly the same error happened in another pull request introducing different feature: #2714

@dhmacs
Copy link

dhmacs commented Dec 15, 2017

Any update on this?

@gaearon gaearon changed the base branch from master to next January 20, 2018 19:40
@steobrien
Copy link

Since #3909 has landed, I believe this change – i.e. simply adding babel-plugin-relay – is sufficient to support Relay. Are there any plans to merge it?

@kassens
Copy link
Member

kassens commented Aug 14, 2018

With the next Relay release (after 1.6.2) and 2.0 of react-scripts, you'll be able to do

import graphql from 'babel-plugin-relay/macro';

graphql`...`

I don't think there needs to be a direct dependency?

@shaneosullivan
Copy link

Thanks @kassens that's great news! Is there any ETA on the Relay release? Or even a days/weeks/months estimate?

@gaearon gaearon closed this Aug 14, 2018
@gaearon
Copy link
Contributor

gaearon commented Aug 14, 2018

Closing per #2343 (comment).

@kassens
Copy link
Member

kassens commented Aug 15, 2018

@shaneosullivan let me try to do an RC this week, so we can test this easily!

@shaneosullivan
Copy link

Awesome, thanks @kassens! Some additional updates to docs and examples would be a huge help too, the current ones on master are fairly confusing

@fernandocalsa
Copy link

Is there anything we can help to speed this up? testing or writing documentation... I have been searching for it for long time

@jeffreypriebe
Copy link

Example of RC usage (expanding from @kassens comments from Aug 14, his example repo):
https://github.com/kassens/relay-cra-example

(For anyone else finding this thread and wanting an example.)

@lock lock bot locked and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.