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

Use RelayRenderer-style API #135

Merged
merged 2 commits into from
Apr 26, 2016
Merged

Use RelayRenderer-style API #135

merged 2 commits into from
Apr 26, 2016

Conversation

taion
Copy link
Member

@taion taion commented Apr 25, 2016

No description provided.

@taion
Copy link
Member Author

taion commented Apr 25, 2016

This replaces #123.

Routes now take a render callback. The semantics try to mimic RelayRenderer, except that it's possible to have props be non-null while ready is false, in the event that new data is being fetched, but none of the params for the route in question have changed.

@taion
Copy link
Member Author

taion commented Apr 25, 2016

This does not address #68. I haven't had time to look at that yet.

@taion
Copy link
Member Author

taion commented Apr 26, 2016

I'm going to merge this for now, then make a few other changes, then cut this as an RC.

@taion taion merged commit 6c36030 into relay-tools:master Apr 26, 2016
@taion taion deleted the relay-renderer branch April 26, 2016 01:03
@taion
Copy link
Member Author

taion commented Apr 26, 2016

This (along with other changes) is out as v0.13.0-rc.1.

@akre54
Copy link

akre54 commented May 2, 2016

How do I actually migrate for this update? What concrete changes do I need to make in my app?

@taion
Copy link
Member Author

taion commented May 2, 2016

@akre54
Copy link

akre54 commented May 2, 2016

Nevermind, I figured it out (I have to pass Relay.Store as the environment prop to Router).

I read the changelog which is how I found this issue.

In the future it would help to have some upgrade guidance on breaking changes like this. I hadn't come across Relay.Environment in my code before so I looked up the docs and tried to create a new Environment but got a slew of other errors. Finally I saw that Relay.Store is the default exported global singleton (which I am familiar with) and tried that. It'd be nice not to have to guess and check for these types of upgrades.

@taion
Copy link
Member Author

taion commented May 2, 2016

The updated README demonstrates the new usage, and the tests show the code in use, plus all the changes are documented in the release notes.

@akre54
Copy link

akre54 commented May 3, 2016

The readme actually doesn't have any mention of environment. And I looked in the tests (as part of the changes to this pull request) but see nothing about Relay.store. I saw a bit about creating the environment, which I tried, but then it complained about a lack of network layer (your tests use a local schema adapter). After a couple more tries I found the solution by looking at the Relay docs.

I'm greatful for all your work, but please have some empathy for your devs. For any breaking change like this it would help to have a bit more guidance from your end to ease the pain on ours. Especially since I'm not intimately familiar with the internals of Relay.

@taion
Copy link
Member Author

taion commented May 3, 2016

The README says explicitly:

pass in a Relay environment to <Router>

Each example in the README also uses environment={Relay.Store} as the pattern most suitable to use cases that don't explicitly require contextual Relay environments.

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

Successfully merging this pull request may close these issues.

2 participants