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

@client(Loader) #39

Merged
merged 8 commits into from
Jun 30, 2016
Merged

@client(Loader) #39

merged 8 commits into from
Jun 30, 2016

Conversation

ericclemmons
Copy link
Owner

I'd love to have a customized way of controlling when components are loading and when they have failed:

@resolve({
  props: {
    users(props) {
      const url = `https://api.github.com/repos/${props.user}/${props.repo}/stargazers`;
      return axios.get(url).then(response => response.data);
    },
  },

  // client side only!
  loading(props) {
    // I can return a React component here and it'll render it in place of the component
    return <div>Loading...</div>
  },

  // server+client
  failed(props) {
    // or I can return an object and it'll pass those props to the component
    return { error: props.error, message: 'Oops' }
  }
})
class Stargazers extends React.Component { }

@iamdustan
Copy link
Contributor

where’s the favorite button when you need it?

@ericclemmons
Copy link
Owner

I was planning on solving this via #2. (I have a project that's in production that'll require this functionality as well!)

However, I don't want to pollute resolve with non-props. (However, I am open to renaming resolve to props in a new version, since it feels right :D)

I've been thinking about this a lot, as you can tell in the novella below!

(See the bottom of the post for Eric's Gut Instinct™)

Loading...

This is primarily for the client, but, in my experience, very specific to the <Handler />, not so much each component.

I, like most React users, started writing code with lots of Loading... messages, but the screen quickly gets overwhelmed with them.

For data-rich components (e.g. "Controllers" that grab the majority of data for the rest of the render tree), this still makes a lot of sense!

  1. Pivot off of propTypes
    • If the base component's propTypes pass (e.g. they didn't specify isRequired), then it's assumed that it supports partial/incomplete data.
    • If isRequired data is missing, the component doesn't render until it's all resolved.
    • If it's not required, then the component can intelligently display the best view until loading completes.
    • This would be the least-invasive API, since it works off of React's existing component API.
  2. Decorators
    • Since @goatslacker mentioned decorators in Adds decorators #31, maybe this could be a clean solution?
    • No idea if this actually works, just throwing it out there. Most likely, it'll be a syntax for another solution.
  3. Full Renders
    • In my experience, you often want to do a "full page render".
    • Async loading of every component can cause major reflows & a jarring experience.
    • It should be possible to let a complete portion of the tree complete before rendering.
    • This would be ideal for a <Handler />
    • Most likely, this would be functionality on top of one of the other solutions, since you still want to define a Loading... view. (e.g. Gmail)

Failures & Errors

It seems to me that, in most cases, rather than rendering the original base component you would:

  1. Redirect to an .../error route.
  2. Render a different component in it's place.

TBH, the team is still determining the best behavior for this. For the moment, the Option 1 solves this well.

The other thing is that, again, in my case, there's a different behavior depending on which prop is missing!

With this in mind, I was considering recommending leveraging the Promise API to handle these cases:

See how an Error can be turned into a Success

The thing is, the following feels wrong to me:

  • Changing the type of object that comes back for a prop, as suggested in Error handling with async resolution #2. (e.g. user suddenly returning an Error object to the view)
  • Passing in hard-coded prop values
  1. On the root options object (above resolve), have hooks for the Promise events resolve, reject, catch for that component. (The signature of these would need to included the failed prop, if possible)
    • onResolve - Pretty much what happens behind the scenes now: fulfill the state.
    • onReject - Ideally for logging & switching routes. Theoretically, we could test the return value for instanceof React.Component and use this to render instead!
    • onCatch / onError - Just like onReject.
    • Do we need this granular of events, or is onError fine?
  2. A wrapping container/decorator that, when the state is rejected in the internals, renders a component.

As you can see, I've been poring over this considerably. If you were to put a gun to my head and say PICK A SOLUTION RIGHT NOW!, it'd be:

  • For loading, pivot off of isRequired.
  • For errors, encourage the use of reject functions and .catch that, when they return a <Component />, this replaces the original base component.

@max-zelinski
Copy link

@ericclemmons waiting desperatly for this feature. Currently we use react-transmit, because it supports loading.

I vote for:

  • For loading, pivot off of isRequired.
  • For errors, encourage the use of reject functions and .catch that, when they return a <Component />, this replaces the original base component.

@ericclemmons
Copy link
Owner

@max-zelinski I'm sorry about that. I'm sprinting on a major project using react-resolver, and definitely have the same needs.

This will be fixed.

@blainekasten
Copy link

@ericclemmons Over a month later, how's that sprint going with this?

@ericclemmons
Copy link
Owner

Mostly here:

https://github.com/ericclemmons/react-resolver/tree/v2/src

Several more updates to come.

@ericclemmons
Copy link
Owner

v2 has a @client(...) decorator for when certain containers aren't resolved on the server, but a dedicated loader will exist then. Cleaning up for future issues...

@ruscoder
Copy link

ruscoder commented Nov 2, 2015

@ericclemmons i am faced with problem. I am using client(Loader)(resolve('data', resolverFn)(MyComponent)); and Loader is not visible while data is loading via resolverFn.
What i am doing wrong?

In src/client.js i see that render method renders Loader when this.state.visible is false and componentDidMount override this.state.visible with true. Therefore: 1. render method renders Loader 2. componentDidMount sets visible to true 3. render method renders only Component (without Loader)

@ericclemmons
Copy link
Owner

@ruscoder Can you provide an example? I have nearly the same code functioning in projects (where the code originated):

@client(Loader)
@resolve("foo", () => load("foo"))
class FooProfile
...

@ruscoder
Copy link

ruscoder commented Nov 3, 2015

@ericclemmons for example i was added to stargazers example (react 0.14) some lines:

  • components/Stargazers.js
const Loader  = React.createClass({
  render: function () {
    console.log('loader');
    return <h1>LOADING</h1>;
  }
});
  • Before first @resolve... i was added decorator @client(Loader)

And as result i dont see loader at page, but i am see loader in console.log.

I think it is because:

      // src/client.js
      componentDidMount() {
        this.setState({ visible: true });
      }

      render() {
        if (!this.state.visible) {
          return Loader ? <Loader /> : null;
        }

        return <Component {...this.props} />;
      }

There render method renders loader (state.visible is false).
After render react engine calls componentDidMount and changes state to visible=true and calls rerender.

What i am doing wrong? Thanks you for fast reply

@EvHaus
Copy link
Contributor

EvHaus commented Mar 2, 2016

Same problem for me too. The loader isn't displayed.

@livemixlove
Copy link

Any updates on this? I'm getting the same issue as @EvNaverniouk . Seems like this almost works.

class Loader extends React.Component {
  render() {
    console.log('loader');
    return <h1>LOADING...</h1>;
  }
}

export default client(Loader)(AgencySingle);

@karlguillotte
Copy link

Any updates on this? I'm getting the same issue.

@livemixlove
Copy link

This project is gathering dust. The community is mostly moving over to Redux. I switched over to Redux and am happy with the decision.

@ericclemmons
Copy link
Owner

@livemixlove is right. I added @pwmckenna as a contributor since I'm no longer using this project.

To be honest, this isn't a hard thing to address or get working. A HoC works pretty well for it (and I have a project in production getting tons of traffic that does just that), but just haven't viewed it.

But, I also feel like a dick for being the only one able to resolve this & doing nothing.

Going to go ahead & keep this tab open and try to resolve this as soon as I can.

For all you Redux users (of which I'm one as well now!), I'm working on a successor to this project that I think is much better architecturally.

@karlguillotte
Copy link

Thanks @ericclemmons and @livemixlove!

I was using this decorator for a particular use case (loading documents from https://prismic.io/). I will move that logic in my Redux store.

@ericclemmons
Copy link
Owner

This project works best with single-page apps that pretty much fetch data based from the URL (or with a classic Flux implementation), since everything is Promise-based.

With Redux or any other single-state library, they don't return promises, so Resolver doesn't really work since it doesn't know when everything's done.

@EvHaus
Copy link
Contributor

EvHaus commented Jun 16, 2016

Sorry to jump in, but since you're discussing Redux, I thought I'd jump in to say that the reason I chose react-resolver, is because I was building a very simple website which needed to fetch some data. I am not using Flux at all. So I was looking for something that could write directly into React state and react-resolver was perfect for that.

Migrating to Redux would be a big undertaking and a bit overkill for my needs. I feel like react-resolver is a good fit for such a use-case.

@ericclemmons
Copy link
Owner

Then I shall fix it. For you @EvNaverniouk.

(Plus, I agree in not adding unnecessary complexity. Redux would make this use-case harder, not easier).

@ericclemmons
Copy link
Owner

Ok...

  1. I'm an idiot. The existing @client is simply a server-side short-circuit the prevents that branch of the tree from rendering on the server. (This was useful for long-running queries or 3rd party fetches that were either not necessary for initial rendering or would hurt perceived performance).
  2. I rewrote it so that it's really just a <Resolver> wrapper that waits until all promises under that component completes and then renders.

I'm on a plane, which worked really well for updating the React v0.14 demo:

loader

And I just confirmed this works with React v0.13 as well!

I'll cut a patch release in a bit. Need to merge in master first...

@ericclemmons ericclemmons changed the title Loading and Failed state? @client(Loader) Jun 29, 2016
@ericclemmons
Copy link
Owner

ericclemmons commented Jun 29, 2016

Whoops! @pwmckenna I'm on a flight right now, so npm install is a no-go for me :)

I won't be home for a few hours, so the last remaining items for this PR are:

  • rm -rf node_modules and npm install the repo + example.
  • npm start and confirm that from / and navigating to the next page, the loader spins on a slow connection (Network tools in Chrome are great for this!)
  • Confirm SSR of same page works.

* origin/master:
  3.0.1
  Adds support for React 15.0.0 (#112)
  Update README.md
  removing the react v0.13 example
  update the changelog with v3.0.0 changesa
  3.0.0
  fix the postversion npm script
  Update README.md
  Update devDependencies
  Use react-dom
@ericclemmons
Copy link
Owner

Hmm, so I think we might want two decorators or something, because we need to accommodate:

  • Skip loading on the server, render purely on the client. (What @client does currently in this PR)
  • Still perform SSR, but when the client needs to load data (e.g. between page transitions), show the loader.

@ericclemmons
Copy link
Owner

Or maybe we just call this PR good, release it so that there's a proper loading thing via @client, then do a separate PR for the SSR version.

@ericclemmons ericclemmons merged commit 659446a into master Jun 30, 2016
@ericclemmons
Copy link
Owner

Alright, this is out (plus more docs!) as v3.0.2.

@ericclemmons ericclemmons mentioned this pull request Jul 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants