Skip to content

[added] Router.AsyncState mixin #125

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

Merged
merged 3 commits into from
Jul 27, 2014
Merged

[added] Router.AsyncState mixin #125

merged 3 commits into from
Jul 27, 2014

Conversation

mjackson
Copy link
Member

This is just a start on the work needed to eventually support server-side rendering (see #57). The other things needed are:

  1. A renderRoutesToString(routes, path) method
  2. Actually mixing this in to Route (or a RouteHandler mixin, see Make Route more extensible #65)

Ideally we can merge these pieces one at a time and make a release when they're all ready.

@ryanflorence
Copy link
Member

what a nice treat to my inbox before going to bed!

* declare a static `getInitialAsyncState` method that fetches state
* for a component after it mounts. This function is given three
* arguments: 1) the current route params, 2) the current query and
* 3) a function that can be used to set state as it is received.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably note here that state will be automatically set to the resolved value, calling setState is just for additional state related to the async operation.

@sophiebits
Copy link
Contributor

The separate setState function weirds me out a bit. I wonder if we can get by without it.

expect(user.state.delayedValue).toEqual('delayed');
expect(user.state.promisedValue).toEqual('promised');
done();
}, 20);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, maybe we should bring in sinon or something else to fake timers (later, this is fine).

@ryanflorence
Copy link
Member

@spicyj how else would you handle the streaming use-case? (But yeah, it is a little weird.)

@sophiebits
Copy link
Contributor

@rpflorence You wouldn't be able to, but is that common? I can't recall a time I've wanted to do that, and it seems to me that it might be easily confused with this.setState.

@ryanflorence
Copy link
Member

I have wanted to be able to update the progress of a long xhr call in ember's router and couldn't, and I know @mjackson has this exact need (streaming) in something he's working on.

@sophiebits
Copy link
Contributor

If only we had Observable in JS… (https://docs.google.com/file/d/0B4PVbLpUIdzoMDR5dWstRllXblU/edit?pli=1).

I don't have any better ideas right now for the streaming use case.

@ryanflorence
Copy link
Member

@spicyj lets just not talk about it much, and if anybody gets a better idea we'll implement it quickly. I'd rather have something weird than nothing at all.

I might even do getInitialAsyncState: function(params, query, soWeird){}

@mjackson
Copy link
Member Author

@spicyj If it makes you feel any better, the setState function is only sugar over the real component's this.setState (we only do a small check to make sure this.isMounted() because it's async). But it's the simplest thing that could work here, and the one that will be most familiar to React devs.

@mjackson
Copy link
Member Author

Anyway, I anticipate that the most common usage will be to just return a hash. This looks exactly like getInitialState, the only difference being the values can be promises.

getInitialAsyncState: function (params, query) {
  return {
    user: getUser(params.userID)
  };
}

@mjackson
Copy link
Member Author

Updated the docs according to @rpflorence feedback and rebased. Let me know if there are any other concerns.

@ryanflorence
Copy link
Member

This is fantastic. Can you smell v1.0?

@mjackson
Copy link
Member Author

:) Please feel free to click the merge button.

Yeah, 1.0 is just around the corner!

ryanflorence added a commit that referenced this pull request Jul 27, 2014
[added] Router.AsyncState mixin
@ryanflorence ryanflorence merged commit 8f22edd into master Jul 27, 2014
@ryanflorence ryanflorence deleted the async-state branch July 27, 2014 17:12
@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants