Skip to content
This repository has been archived by the owner on Jan 27, 2021. It is now read-only.

Updated react-redux bindings to v6 and updated examples #103

Closed
wants to merge 1 commit into from
Closed

Updated react-redux bindings to v6 and updated examples #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6,041 changes: 3,647 additions & 2,394 deletions examples/real-world/package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions examples/real-world/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,23 @@
"version": "3.0.1",
"private": true,
"devDependencies": {
"react-scripts": "^2.0.5",
"redux-devtools": "^3.4.1",
"react-scripts": "^2.1.5",
"redux-devtools": "^3.5.0",
"redux-devtools-dock-monitor": "^1.1.3",
"redux-devtools-log-monitor": "^1.4.0",
"redux-logger": "^3.0.6"
},
"dependencies": {
"connected-react-router": "^4.5.0",
"connected-react-router": "^6.3.1",
"history": "^4.7.2",
"humps": "^2.0.1",
"lodash": "^4.17.11",
"normalizr": "^3.3.0",
"prop-types": "^15.6.2",
"react": "^16.6.0",
"react-dom": "^16.6.0",
"react-redux": "^5.1.0",
"react-redux-subspace": "^2.5.0",
"prop-types": "^15.7.2",
"react": "^16.8.2",
"react-dom": "^16.8.2",
"react-redux": "^6.0.0",
"react-redux-subspace": "^3.0.1",
"react-router": "^4.3.1",
"react-router-dom": "^4.3.1",
"redux": "^4.0.1",
Expand Down
6 changes: 4 additions & 2 deletions examples/real-world/src/root/reducers/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { combineReducers } from 'redux'
import { namespaced } from 'redux-subspace'
import { connectRouter } from 'connected-react-router'
import { reducer as app } from '../../app'
import configuration from './configuration'

export default combineReducers({
export default history => combineReducers({
app: namespaced('app')(app),
configuration
configuration,
router: connectRouter(history)
})
12 changes: 8 additions & 4 deletions examples/real-world/src/root/routes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,26 @@
import React from 'react'
import { Route, Switch } from 'react-router'
import { SubspaceProvider } from 'react-redux-subspace'

// work around for lerna/npm link issue for local development
import { ReactReduxContext } from 'react-redux'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You'll see this a lot in the review. When lerna links the packages using npm link they end up with different instances of react-redux so the React.createContext(...) calls create a different context so react-redux-subspace cannot access the store from the examples Provider. This follows the "BYO contextconcept fromreact-redux` but provides the actual context instance from the example's version.

This works, but it's a bit gross and not really how you would use in a real app, which makes having it in all the examples a bit annoying.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is one annoying thing about reacts new context api.

Could we have a post install script that removes the nested react-redux packages in node_modules so they all resolve to the same one?

At least then the fix is hidden away in the package.json and not in the example code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, that would break the build of the actual package... Ignore that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue with that is the file is not in the same directory tree, so the node package resolution does not find the package at all. I have done a trick in the past to npm link the troublesome package back to the other's as a post-installation task, but I found it to be slow and required a bit of hacking about to get working. At least this is actual functionality offered by both react-redux and now redux-subspace, it just sucks a bit that it's the defacto example.

Copy link
Collaborator

@jpeyper jpeyper Feb 20, 2019 •
edited

Choose a reason for hiding this comment

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

At least it won't break if someone uses it in production code (it'll be the same as the default value)

Maybe adjust the comment a bit to make it more obvious?

"// Note: please don't copy this line (and associated prop usage) to your project, you are unlikely to need to do this for standard subspace usage.
// This is caused by lerna using npm link..."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lol at autocorrect, but I like the idea.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That'll teach me for reviewing on my phone.


import { App } from '../app'
import { UserPage } from '../userPage'
import { RepoPage } from '../repoPage'

const Routes = () => (
<Route path="/" render={props => (
<SubspaceProvider mapState={(state) => state.app} namespace="app">
<SubspaceProvider mapState={(state) => state.app} namespace="app" context={ReactReduxContext}>
<App {...props}>
<Switch>
<Route path="/:login/:name" render={props => (
<SubspaceProvider mapState={(state) => state.repoPage} namespace="repoPage">
<SubspaceProvider mapState={(state) => state.repoPage} namespace="repoPage" context={ReactReduxContext}>
<RepoPage {...props} />
</SubspaceProvider>
)} />
<Route path="/:login" render={props => console.log(props) || (
<SubspaceProvider mapState={(state) => state.userPage} namespace="userPage">
<Route path="/:login" render={props => (
<SubspaceProvider mapState={(state) => state.userPage} namespace="userPage" context={ReactReduxContext}>
<UserPage {...props} />
</SubspaceProvider>
)} />
Expand Down
6 changes: 3 additions & 3 deletions examples/real-world/src/root/store/configureStore.dev.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { createStore, compose } from 'redux'
import { applyMiddleware, applyToRoot } from 'redux-subspace'
import thunk from 'redux-thunk'
import { connectRouter, routerMiddleware } from 'connected-react-router'
import { routerMiddleware } from 'connected-react-router'
import { createLogger } from 'redux-logger'
import wormhole from 'redux-subspace-wormhole'
import api from '../../common/middleware/api'
import rootReducer from '../reducers'
import createRootReducer from '../reducers'
import DevTools from '../containers/DevTools'

const configureStore = history => {
const store = createStore(
connectRouter(history)(rootReducer),
createRootReducer(history),
compose(
applyMiddleware(
thunk,
Expand Down
6 changes: 3 additions & 3 deletions examples/real-world/src/root/store/configureStore.prod.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { createStore } from 'redux'
import { applyMiddleware } from 'redux-subspace'
import thunk from 'redux-thunk'
import { connectRouter, routerMiddleware } from 'connected-react-router'
import { routerMiddleware } from 'connected-react-router'
import wormhole from 'redux-subspace-wormhole'
import api from '../../common/middleware/api'
import rootReducer from '../reducers'
import createRootReducer from '../reducers'

const configureStore = history => createStore(
connectRouter(history)(rootReducer),
createRootReducer(history),
applyMiddleware(
thunk,
api,
Expand Down