You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by timmak <gi...@git.apache.org> on 2017/02/14 14:40:33 UTC
[GitHub] couchdb-fauxton pull request #849: Currently reducers seem to be handled qui...
GitHub user timmak opened a pull request:
https://github.com/apache/couchdb-fauxton/pull/849
Currently reducers seem to be handled quite strangely
this is a slightly better work around for now
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/timmak/couchdb-fauxton better-reducer-handling
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-fauxton/pull/849.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #849
----
commit a38f466c9f920a0fa67576c564252ea5536add10
Author: Tim Pinington <ti...@gmail.com>
Date: 2017-02-14T14:39:32Z
Currently reducers seem to be handled quite strangely this is a better work around
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request #849: Currently reducers seem to be handled qui...
Posted by timmak <gi...@git.apache.org>.
Github user timmak commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/849#discussion_r101054922
--- Diff: app/addons/permissions/container/PermissionsContainer.js ---
@@ -24,14 +24,14 @@ import {
} from '../reducers';
-const mapStateToProps = (state) => {
+const mapStateToProps = ({permissions}) => {
return {
--- End diff --
Permission are now named space in the reducer so with need to pick permissions from the whole state
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request #849: Currently reducers seem to be handled qui...
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/849#discussion_r101262321
--- Diff: app/core/base.js ---
@@ -157,6 +157,10 @@ FauxtonAPI.setSession = function (newSession) {
return FauxtonAPI.session.fetchUser();
};
-FauxtonAPI.reducers = [];
+FauxtonAPI.reducers = {};
+
+FauxtonAPI.addReducers = (reducers) => {
+ FauxtonAPI.reducers = Object.assign({}, FauxtonAPI.reducers, reducers);
+};
--- End diff --
I haven't tested if this will work or not. What do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request #849: Currently reducers seem to be handled qui...
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/849#discussion_r101262280
--- Diff: app/core/base.js ---
@@ -157,6 +157,10 @@ FauxtonAPI.setSession = function (newSession) {
return FauxtonAPI.session.fetchUser();
};
-FauxtonAPI.reducers = [];
+FauxtonAPI.reducers = {};
+
+FauxtonAPI.addReducers = (reducers) => {
+ FauxtonAPI.reducers = Object.assign({}, FauxtonAPI.reducers, reducers);
+};
--- End diff --
This is really cool. What about if you used the [spread operator](https://babeljs.io/docs/plugins/transform-object-rest-spread/) here. Then this should be:
```
FauxtonAPI.addReducers = (reducers) => {
FauxtonAPI.reducers = {
...FauxtonAPI.reducers,
reducers
};
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request #849: Currently reducers seem to be handled qui...
Posted by timmak <gi...@git.apache.org>.
Github user timmak commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/849#discussion_r101274745
--- Diff: app/core/base.js ---
@@ -157,6 +157,10 @@ FauxtonAPI.setSession = function (newSession) {
return FauxtonAPI.session.fetchUser();
};
-FauxtonAPI.reducers = [];
+FauxtonAPI.reducers = {};
+
+FauxtonAPI.addReducers = (reducers) => {
+ FauxtonAPI.reducers = Object.assign({}, FauxtonAPI.reducers, reducers);
+};
--- End diff --
Unfortunately the build and linting isn't currently setup to whole object spread syntax, though this is a lot nicer
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton issue #849: Currently reducers seem to be handled quite stra...
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the issue:
https://github.com/apache/couchdb-fauxton/pull/849
Thanks @timmak I've merged this in. Can you close this PR.
For future reference don't merge from master into your working branch. Rather rebase master into your PR branch. And then when getting your code into master do a merge then. Otherwise it makes a mess of the git history. I've fixed up the git history and squished your commits into one.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton issue #849: Currently reducers seem to be handled quite stra...
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the issue:
https://github.com/apache/couchdb-fauxton/pull/849
+1 this is really cool. Could you rebase from master, I think we had an issue with the docker image. The tests should pass after that and then I can merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton issue #849: Currently reducers seem to be handled quite stra...
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the issue:
https://github.com/apache/couchdb-fauxton/pull/849
thanks for the clarifying comments. I can't personally review the work, we'll need @garrensmith
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request #849: Currently reducers seem to be handled qui...
Posted by timmak <gi...@git.apache.org>.
Github user timmak commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/849#discussion_r101279779
--- Diff: app/core/base.js ---
@@ -157,6 +157,10 @@ FauxtonAPI.setSession = function (newSession) {
return FauxtonAPI.session.fetchUser();
};
-FauxtonAPI.reducers = [];
+FauxtonAPI.reducers = {};
+
+FauxtonAPI.addReducers = (reducers) => {
+ FauxtonAPI.reducers = Object.assign({}, FauxtonAPI.reducers, reducers);
+};
--- End diff --
Ok all done should be good to go once tests have passed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request #849: Currently reducers seem to be handled qui...
Posted by timmak <gi...@git.apache.org>.
Github user timmak commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/849#discussion_r101054470
--- Diff: app/main.js ---
@@ -57,24 +57,10 @@ $(document).on("click", "a:not([data-bypass])", function (evt) {
}
});
-function getReducers (r) {
-
- if (!r.length) {
- return function () {};
- }
-
- return FauxtonAPI.reducers.reduce((el, acc) => {
- acc[el] = el;
- return acc;
- }, {});
-}
-
-const reducer = getReducers(FauxtonAPI.reducers);
-
const middlewares = [thunk];
const store = createStore(
- reducer,
+ combineReducers(FauxtonAPI.reducers),
--- End diff --
Instead of having a single reducer we use redux combineReducer so we can name space the different reducers
http://redux.js.org/docs/api/combineReducers.html
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request #849: Currently reducers seem to be handled qui...
Posted by timmak <gi...@git.apache.org>.
Github user timmak commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/849#discussion_r101055093
--- Diff: app/addons/permissions/base.js ---
@@ -12,11 +12,13 @@
import FauxtonAPI from "../../core/api";
import Permissions from "./routes";
-import reducer from './reducers';
+import reducers from './reducers';
import "./assets/less/permissions.less";
Permissions.initialize = function () {};
-FauxtonAPI.reducers.push(reducer);
+FauxtonAPI.addReducers({
+ permissions: reducers
+});
--- End diff --
We declare the name and the reducer to use, using the add reducer api
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton issue #849: Currently reducers seem to be handled quite stra...
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the issue:
https://github.com/apache/couchdb-fauxton/pull/849
This description is not very helpful.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request #849: Currently reducers seem to be handled qui...
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/849#discussion_r101277979
--- Diff: app/core/base.js ---
@@ -157,6 +157,10 @@ FauxtonAPI.setSession = function (newSession) {
return FauxtonAPI.session.fetchUser();
};
-FauxtonAPI.reducers = [];
+FauxtonAPI.reducers = {};
+
+FauxtonAPI.addReducers = (reducers) => {
+ FauxtonAPI.reducers = Object.assign({}, FauxtonAPI.reducers, reducers);
+};
--- End diff --
Can't we just add the npm package and it will work? I would be happy for you to do that in this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request #849: Currently reducers seem to be handled qui...
Posted by timmak <gi...@git.apache.org>.
Github user timmak closed the pull request at:
https://github.com/apache/couchdb-fauxton/pull/849
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request #849: Currently reducers seem to be handled qui...
Posted by timmak <gi...@git.apache.org>.
Github user timmak commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/849#discussion_r101054763
--- Diff: app/core/base.js ---
@@ -157,6 +157,10 @@ FauxtonAPI.setSession = function (newSession) {
return FauxtonAPI.session.fetchUser();
};
-FauxtonAPI.reducers = [];
+FauxtonAPI.reducers = {};
+
+FauxtonAPI.addReducers = (reducers) => {
+ FauxtonAPI.reducers = Object.assign({}, FauxtonAPI.reducers, reducers);
+};
--- End diff --
This not the most elegant solution but reducers are named space by an object so when we add reducer to the store we merge it with previous reducers object and reassign it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---