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

Action Creators: unwrap Promise with Action #215

Closed
alexeyraspopov opened this Issue Jul 5, 2015 · 5 comments

Comments

2 participants
@alexeyraspopov
Contributor

alexeyraspopov commented Jul 5, 2015

The idea to describe AC as pure functions is good but it has one disadvantage. It looks good only for sync stuff.

function addTodo() {
    return { type: TODO_ADDED, title: "rule the world" };
}

The main purpose of using AC is to isolate asynchronous stuff from other parts of architecture (like view layer and stores layer). And based on my experience on projects with server side (or another types of WebAPI that you can use) you will create a lot of async AC. So it would be better if the developer will be able to write async AC easy just like sync one.

Currently, I need to wrap all my async operations and use dispatch function as a side effect.

function loadProjects(token) {
  return dispatch => {
    WebAPI.projects(token)
        .then(projects => dispatch({ type: PROJECTS_LOADED, data: projects }))
  };
}

Lets accept Promises and unwrap them to get Action instance

function loadProjects(token) {
  return WebAPI.projects(token)
        .then(projects => ({ type: PROJECTS_LOADED, data: projects }));
}

I think it's a way to full consistency and purity.

@alexeyraspopov

This comment has been minimized.

Show comment
Hide comment
@alexeyraspopov

alexeyraspopov Jul 5, 2015

Contributor

Sorry I didn't figure out what branch I can use to add this feature. But I'll be glad to make a PR.

Contributor

alexeyraspopov commented Jul 5, 2015

Sorry I didn't figure out what branch I can use to add this feature. But I'll be glad to make a PR.

@acdlite

This comment has been minimized.

Show comment
Hide comment
@acdlite

acdlite Jul 5, 2015

Collaborator

This can and should be implemented outside of the core using middleware. You can read more about middleware here (docs improvements are impending).

Check out redux-promise, which looks like it fits your needs: https://github.com/acdlite/redux-promise

Collaborator

acdlite commented Jul 5, 2015

This can and should be implemented outside of the core using middleware. You can read more about middleware here (docs improvements are impending).

Check out redux-promise, which looks like it fits your needs: https://github.com/acdlite/redux-promise

@acdlite acdlite closed this Jul 5, 2015

@alexeyraspopov

This comment has been minimized.

Show comment
Hide comment
@alexeyraspopov

alexeyraspopov Jul 5, 2015

Contributor

What the purpose not to implement it in the core? It can be straightforward change that will only simplify development workflow. Also, I expected this feature as an obvious solution that should be available without any additional plugins/middlewares/another-shittiest-type-of-modules.

Contributor

alexeyraspopov commented Jul 5, 2015

What the purpose not to implement it in the core? It can be straightforward change that will only simplify development workflow. Also, I expected this feature as an obvious solution that should be available without any additional plugins/middlewares/another-shittiest-type-of-modules.

@acdlite

This comment has been minimized.

Show comment
Hide comment
@acdlite

acdlite Jul 5, 2015

Collaborator

I agree it's a natural feature that most Redux apps will probably want to have, but once we put it into the core, everyone will start bikeshedding exactly how it should work, which is what happened for me with Flummox. We're trying to keep the core as minimal and flexible as possible so we can iterate quickly and allow others to build on top of it.

Also, even if we did implement it in the core, it'd be in exactly the same way we're recommending now — as a layer on top of dispatch(). Check out this PR (which will likely be merged before 1.0) for more information on why the API is designed this way.

#213

Collaborator

acdlite commented Jul 5, 2015

I agree it's a natural feature that most Redux apps will probably want to have, but once we put it into the core, everyone will start bikeshedding exactly how it should work, which is what happened for me with Flummox. We're trying to keep the core as minimal and flexible as possible so we can iterate quickly and allow others to build on top of it.

Also, even if we did implement it in the core, it'd be in exactly the same way we're recommending now — as a layer on top of dispatch(). Check out this PR (which will likely be merged before 1.0) for more information on why the API is designed this way.

#213

@acdlite

This comment has been minimized.

Show comment
Hide comment
@acdlite

acdlite Jul 5, 2015

Collaborator

As @gaearon said once, (I can't remember where... probably Slack) we're aiming to be like the Koa of Flux libraries. Eventually, once the community is more mature, the plan is to maintain a collection of "blessed" plugins and extensions, possibly under a reduxjs GitHub organization.

Collaborator

acdlite commented Jul 5, 2015

As @gaearon said once, (I can't remember where... probably Slack) we're aiming to be like the Koa of Flux libraries. Eventually, once the community is more mature, the plan is to maintain a collection of "blessed" plugins and extensions, possibly under a reduxjs GitHub organization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment