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

Fix broken relay-experimental imports; Add Record: RelayModernRecord to relay-runtime API #2847

Closed

Conversation

@babangsund
Copy link
Contributor

babangsund commented Sep 9, 2019

Suggested fix for #2845

@babangsund babangsund force-pushed the babangsund:relay-experimental-paths branch 3 times, most recently from e9f179b to 77e505f Sep 10, 2019
Copy link

facebook-github-bot left a comment

@jstejada has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@@ -11,7 +11,7 @@

'use strict';

const ReactRelayContext = require('react-relay/ReactRelayContext');
const {ReactRelayContext} = require('react-relay');

This comment has been minimized.

Copy link
@jstejada

jstejada Sep 13, 2019

Contributor

the problem here is that we don't actually want to bring in all of 'react-relay' in our web bundle, let me see if I can get this to work

This comment has been minimized.

Copy link
@babangsund

babangsund Sep 13, 2019

Author Contributor

Hadn't considered this aspect. 🤔

EDIT:

Oh I see, so relay-experimental is obviously a replacement/next of react-relay.

Couldn't you justify just re-defining ReactRelayContext(RelayExperimentalContext?) if they aren't meant to be used together anyway?

I don't think it'd break backwards compatibility either... 🙂

This comment has been minimized.

Copy link
@jstejada

jstejada Sep 13, 2019

Contributor

I think we still want to be compatible with the apis in react-relay, so you can use both apis together and migrate incrementally to hooks if you want (without having to convert everything to hooks at once), which means that we need to use the same context; but for cases where we're exclusively using relay-experimental we don't to bring in all of react-relay.

I think I might have a solution for this though

This comment has been minimized.

Copy link
@babangsund

babangsund Sep 13, 2019

Author Contributor

Okay. Looking forward to being blown away. 😉

…to relay-runtime API
@babangsund babangsund force-pushed the babangsund:relay-experimental-paths branch from 77e505f to 3fdce8e Sep 13, 2019
Copy link

facebook-github-bot left a comment

@jstejada has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@babangsund babangsund deleted the babangsund:relay-experimental-paths branch Sep 13, 2019
@facebook-github-bot

This comment has been minimized.

Copy link

facebook-github-bot commented Sep 13, 2019

@jstejada merged this pull request in bf78647.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.