From 71ab7c33e07d2af2bf5885fe5c29140837da52bd Mon Sep 17 00:00:00 2001 From: Vesa Luusua Date: Fri, 27 Jan 2017 00:55:01 +0200 Subject: [PATCH 1/6] Call component.loadData on server side (and client) --- package.json | 3 +- server/index.js | 22 ++++++- src/containers/SearchPage/SearchPage.js | 87 +++++++++++++++---------- src/index.js | 3 + src/routesConfiguration.js | 41 +++++++++++- 5 files changed, 118 insertions(+), 38 deletions(-) diff --git a/package.json b/package.json index c5d7aeef..e4420155 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,8 @@ "redux-saga": "^0.14.3", "sanitize.css": "^4.1.0", "sharetribe-scripts": "0.8.6", - "source-map-support": "^0.4.11" + "source-map-support": "^0.4.11", + "url": "^0.11.0" }, "devDependencies": { "nodemon": "^1.11.0", diff --git a/server/index.js b/server/index.js index a2cc2d6c..5be3f781 100644 --- a/server/index.js +++ b/server/index.js @@ -22,6 +22,7 @@ const compression = require('compression'); const path = require('path'); const fs = require('fs'); const qs = require('qs'); +const url = require('url'); const _ = require('lodash'); const React = require('react'); const { createServerRenderContext } = require('react-router'); @@ -33,7 +34,9 @@ const buildPath = path.resolve(__dirname, '..', 'build'); const manifestPath = path.join(buildPath, 'asset-manifest.json'); const manifest = require(manifestPath); const mainJsPath = path.join(buildPath, manifest['main.js']); -const renderApp = require(mainJsPath).default; +const mainJs = require(mainJsPath); +const renderApp = mainJs.default; +const matchPathname = mainJs.matchPathname; // The HTML build file is generated from the `public/index.html` file // and used as a template for server side rendering. The application @@ -66,8 +69,21 @@ const template = _.template(indexHtml, { escape: reNoMatch, }); -function render(url, context, preloadedState) { - const { head, body } = renderApp(url, context, preloadedState); +function render(requestUrl, context, preloadedState) { + const { head, body } = renderApp(requestUrl, context, preloadedState); + + const pathname = url.parse(requestUrl).pathname; + const { matchedRoutes, params } = matchPathname(pathname); + const component = matchedRoutes[0].component; + + if (component.loadData) { + component + .loadData() + .then((val) => { + console.log('Data fetch resolved', val); + }); + } + // Preloaded state needs to be passed for client side too. // For security reasons we ensure that preloaded state is considered as a string diff --git a/src/containers/SearchPage/SearchPage.js b/src/containers/SearchPage/SearchPage.js index 5aeb1d17..938a512d 100644 --- a/src/containers/SearchPage/SearchPage.js +++ b/src/containers/SearchPage/SearchPage.js @@ -1,4 +1,4 @@ -import React, { PropTypes } from 'react'; +import React, { Component, PropTypes } from 'react'; import classNames from 'classnames'; import { connect } from 'react-redux'; import { includes } from 'lodash'; @@ -39,41 +39,53 @@ const fakeListings = [ }, ]; -export const SearchPageComponent = props => { - const { tab } = props; - const selectedTab = includes(['filters','listings', 'map'], tab) - ? tab - : 'listings'; +export class SearchPageComponent extends Component { - const filtersClassName = classNames(css.filters, { - [css.open]: selectedTab === 'filters', - }); - const listingsClassName = classNames(css.filters, { - [css.open]: selectedTab === 'listings', - }); - const mapClassName = classNames(css.filters, { - [css.open]: selectedTab === 'map', - }); + componentWillMount() { + /* eslint-disable no-console */ + console.log('Client loads data'); + SearchPageComponent.loadData().then((val) => { + console.log('Client fetched data', val); + }); + /* eslint-enable no-console */ + } - return ( - -
-
- + render() { + const { tab } = this.props; + const selectedTab = includes(['filters','listings', 'map'], tab) + ? tab + : 'listings'; + + const filtersClassName = classNames(css.filters, { + [css.open]: selectedTab === 'filters', + }); + const listingsClassName = classNames(css.filters, { + [css.open]: selectedTab === 'listings', + }); + const mapClassName = classNames(css.filters, { + [css.open]: selectedTab === 'map', + }); + + return ( + +
+
+ +
+
+ + {fakeListings.map(l => )} + +
+
+ + {fakeListings.map(l => )} + +
-
- - {fakeListings.map(l => )} - -
-
- - {fakeListings.map(l => )} - -
-
- - ); + + ); + } }; SearchPageComponent.defaultProps = { tab: 'listings' }; @@ -82,6 +94,15 @@ const { string } = PropTypes; SearchPageComponent.propTypes = { tab: string }; +SearchPageComponent.loadData = () => ( + new Promise((resolve) => ( + // also node has a 'setTimeout' -> it's good for testing async call. + setTimeout(() => { + resolve(fakeListings); + }, (Math.random() * 2000) + 1000) + )) +); + /** * Container functions. * Since we add this to global store state with combineReducers, this will only get partial state diff --git a/src/index.js b/src/index.js index 30f46bd5..7f782c02 100644 --- a/src/index.js +++ b/src/index.js @@ -15,6 +15,7 @@ import React from 'react'; import ReactDOM from 'react-dom'; import { ClientApp, renderApp } from './app'; import configureStore from './store'; +import { matchLocation } from './routesConfiguration'; import './index.css'; @@ -29,3 +30,5 @@ if (typeof window !== 'undefined') { // Export the function for server side rendering. export default renderApp; + +export { matchLocation }; diff --git a/src/routesConfiguration.js b/src/routesConfiguration.js index a57a7144..0c1367a6 100644 --- a/src/routesConfiguration.js +++ b/src/routesConfiguration.js @@ -1,6 +1,10 @@ import React from 'react'; import { find } from 'lodash'; import { Redirect } from 'react-router'; + +// This will change to `matchPath` soonish +import matchPattern from 'react-router/matchPattern' + import pathToRegexp from 'path-to-regexp'; import { AuthenticationPage, @@ -269,7 +273,42 @@ const toPathByRouteName = (nameToFind, routes) => { const pathByRouteName = (nameToFind, routes, params = {}) => toPathByRouteName(nameToFind, routes)(params); +/** + * matchRoutesToLocation helps to figure out which routes are related to given location. + */ +const matchRoutesToLocation = (routes, location, matchedRoutes=[], params={}) => { + const parameters = { ...params }; + routes.forEach((route) => { + const { exactly = false } = route; + const match = !route.pattern ? true : matchPattern(route.pattern, location, exactly); + + if (match) { + matchedRoutes.push(route); + + if (match.params) { + Object.keys(match.params).forEach(key => { parameters[key] = match.params[key]; }); + } + } + + if (route.routes) { + matchRoutesToLocation(route.routes, location, matchedRoutes, parameters); + } + }); + + return { matchedRoutes, params: parameters }; +} + +const matchPathnameCreator = routes => ( + (location, matchedRoutes=[], params={}) => + matchRoutesToLocation(routes, { pathname: location }, matchedRoutes, params) +); + +/** + * matchRoutesToLocation helps to figure out which routes are related to given location. + */ +const matchPathname = matchPathnameCreator(routesConfiguration); + // Exported helpers -export { findRouteByName, flattenRoutes, toPathByRouteName, pathByRouteName }; +export { findRouteByName, flattenRoutes, matchPathname, toPathByRouteName, pathByRouteName }; export default routesConfiguration; From 3c597ea2cf338f4525005c23ec25c429f650f3ec Mon Sep 17 00:00:00 2001 From: Vesa Luusua Date: Thu, 2 Feb 2017 20:00:19 +0200 Subject: [PATCH 2/6] Template function didn't understood stuff inside script tags. --- public/index.html | 2 +- server/index.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/public/index.html b/public/index.html index d095f837..edc9c714 100644 --- a/public/index.html +++ b/public/index.html @@ -5,7 +5,7 @@ - +
diff --git a/server/index.js b/server/index.js index 5be3f781..233a6760 100644 --- a/server/index.js +++ b/server/index.js @@ -90,7 +90,7 @@ function render(requestUrl, context, preloadedState) { // by replacing '<' character with its unicode equivalent. // http://redux.js.org/docs/recipes/ServerRendering.html#security-considerations const preloadedStateScript = ` - window.__PRELOADED_STATE__ = ${JSON.stringify(preloadedState).replace(/window.__PRELOADED_STATE__ = ${JSON.stringify(preloadedState).replace(/ `; return template({ title: head.title.toString(), preloadedStateScript, body }); From 2a9c9e103bbdc9d57c2f74320fa2e7989909fecb Mon Sep 17 00:00:00 2001 From: Vesa Luusua Date: Fri, 3 Feb 2017 00:10:20 +0200 Subject: [PATCH 3/6] Fetch data with saga (both server and client) --- server/index.js | 75 ++++-- src/containers/SearchPage/SearchPage.ducks.js | 64 ++++- src/containers/SearchPage/SearchPage.js | 75 ++---- src/containers/SearchPage/SearchPage.test.js | 6 +- .../__snapshots__/SearchPage.test.js.snap | 255 +----------------- src/containers/SearchPage/fakeData.js | 26 ++ src/index.js | 6 +- src/routesConfiguration.js | 23 +- src/sagas.js | 3 +- src/store.js | 8 +- 10 files changed, 189 insertions(+), 352 deletions(-) create mode 100644 src/containers/SearchPage/fakeData.js diff --git a/server/index.js b/server/index.js index 233a6760..944d5920 100644 --- a/server/index.js +++ b/server/index.js @@ -27,6 +27,7 @@ const _ = require('lodash'); const React = require('react'); const { createServerRenderContext } = require('react-router'); const auth = require('./auth'); +const sagaEffects = require('redux-saga/effects'); // Construct the bundle path where the server side rendering function // can be imported. @@ -37,6 +38,7 @@ const mainJsPath = path.join(buildPath, manifest['main.js']); const mainJs = require(mainJsPath); const renderApp = mainJs.default; const matchPathname = mainJs.matchPathname; +const configureStore = mainJs.configureStore; // The HTML build file is generated from the `public/index.html` file // and used as a template for server side rendering. The application @@ -69,28 +71,46 @@ const template = _.template(indexHtml, { escape: reNoMatch, }); -function render(requestUrl, context, preloadedState) { - const { head, body } = renderApp(requestUrl, context, preloadedState); - +function fetchInitialState(requestUrl) { const pathname = url.parse(requestUrl).pathname; const { matchedRoutes, params } = matchPathname(pathname); - const component = matchedRoutes[0].component; + const initialFetches = _ + .chain(matchedRoutes) + .filter(r => r.loadData && r.loadData.fetchData) + .map(r => sagaEffects.fork(r.loadData.fetchData)) + .value(); - if (component.loadData) { - component - .loadData() - .then((val) => { - console.log('Data fetch resolved', val); - }); + const fetchInitialData = function* fetchInitialData() { + yield initialFetches; + }; + + if (initialFetches.length > 0) { + const fetchPromise = new Promise((resolve, reject) => { + const store = configureStore({}); + store.runSaga(fetchInitialData).done + .then(() => { + resolve(store.getState()); + }) + .catch(e => { + reject(e); + }); + store.close(); + }); + return fetchPromise; } + return Promise.resolve({}); +} +function render(requestUrl, context, preloadedState) { + const { head, body } = renderApp(requestUrl, context, preloadedState); // Preloaded state needs to be passed for client side too. // For security reasons we ensure that preloaded state is considered as a string // by replacing '<' character with its unicode equivalent. // http://redux.js.org/docs/recipes/ServerRendering.html#security-considerations + const serializedState = JSON.stringify(preloadedState).replace(/window.__PRELOADED_STATE__ = ${JSON.stringify(preloadedState).replace(/ + `; return template({ title: head.title.toString(), preloadedStateScript, body }); @@ -120,21 +140,26 @@ app.get('*', (req, res) => { const filters = qs.parse(req.query); // TODO fetch this asynchronously - const preloadedState = { search: { filters } }; + fetchInitialState(req.url) + .then(preloadedState => { + const html = render(req.url, context, preloadedState); + const result = context.getResult(); - const html = render(req.url, context, preloadedState); - const result = context.getResult(); - - if (result.redirect) { - res.redirect(result.redirect.pathname); - } else if (result.missed) { - // Do a second render pass with the context to clue - // components into rendering this time. - // See: https://react-router.now.sh/ServerRouter - res.status(404).send(render(req.url, context, preloadedState)); - } else { - res.send(html); - } + if (result.redirect) { + res.redirect(result.redirect.pathname); + } else if (result.missed) { + // Do a second render pass with the context to clue + // components into rendering this time. + // See: https://react-router.now.sh/ServerRouter + res.status(404).send(render(req.url, context, preloadedState)); + } else { + res.send(html); + } + }) + .catch(e => { + console.error(e.message); + res.status(500).send(e.message); + }); }); app.listen(PORT, () => { diff --git a/src/containers/SearchPage/SearchPage.ducks.js b/src/containers/SearchPage/SearchPage.ducks.js index f977d4a7..9f4bfa8d 100644 --- a/src/containers/SearchPage/SearchPage.ducks.js +++ b/src/containers/SearchPage/SearchPage.ducks.js @@ -4,22 +4,78 @@ * https://github.com/erikras/ducks-modular-redux */ import { unionWith, isEqual } from 'lodash'; +import { delay } from 'redux-saga'; +import { call, take, put } from 'redux-saga/effects'; +import { fakeListings } from './fakeData'; + +// ================ Action types ================ // -// Actions export const ADD_FILTER = 'app/SearchPage/ADD_FILTER'; +export const LOAD_LISTINGS = 'app/SearchPage/LOAD_LISTINGS'; +export const LOAD_LISTINGS_SUCCESS = 'app/SearchPage/LOAD_LISTINGS_SUCCESS'; +export const LOAD_LISTINGS_FAILURE = 'app/SearchPage/LOAD_LISTINGS_FAILURE'; -// Reducer -export default function reducer(state = {}, action = {}) { +// ================ Reducer ================ // + +export const initialState = { + filters: [], + initialListingsLoaded: false, + listings: [], + loadingListings: false, +}; + +export default function reducer(state = initialState, action = {}) { const { type, payload } = action; switch (type) { case ADD_FILTER: { const stateFilters = state.filters || []; return { ...state, ...{ filters: unionWith(stateFilters, [payload], isEqual) } }; } + case LOAD_LISTINGS: + return { ...state, loadingListings: true, initialListingsLoaded: false }; + case LOAD_LISTINGS_SUCCESS: + return { ...state, loadingListings: false, initialListingsLoaded: true, listings: payload }; + case LOAD_LISTINGS_FAILURE: + return { ...state, loadingListings: false, loadListingError: payload }; default: return state; } } -// Action Creators +// ================ Action creators ================ // + export const addFilter = (key, value) => ({ type: ADD_FILTER, payload: { [key]: value } }); +export const loadListings = () => ({ type: LOAD_LISTINGS, payload: {} }); +export const loadListingsSuccess = listings => ({ type: LOAD_LISTINGS_SUCCESS, payload: listings }); +export const loadListingsFailure = error => ({ + type: LOAD_LISTINGS_FAILURE, + payload: error, + error: true, +}); + +// ================ Worker sagas ================ // + +export function fetchListings() { + const randomTime = Math.random() * 2000; + const fakeResponseTime = 1000 + randomTime; + return delay(fakeResponseTime, { response: fakeListings }); +} + +export function* callFetchListings() { + const { response, error } = yield call(fetchListings); + if (response) { + yield put(loadListingsSuccess(response)); + } else { + yield put(loadListingsFailure(error)); + } +} + +// ================ Watcher sagas ================ // + +export function* watchLoadListings() { + // eslint-disable-next-line no-constant-condition + while (true) { + yield take(LOAD_LISTINGS); + yield call(callFetchListings); + } +} diff --git a/src/containers/SearchPage/SearchPage.js b/src/containers/SearchPage/SearchPage.js index 938a512d..590e58f5 100644 --- a/src/containers/SearchPage/SearchPage.js +++ b/src/containers/SearchPage/SearchPage.js @@ -3,7 +3,7 @@ import classNames from 'classnames'; import { connect } from 'react-redux'; import { includes } from 'lodash'; import { addFlashNotification } from '../../ducks/FlashNotification.ducks'; -import { addFilter } from './SearchPage.ducks'; +import { addFilter, callFetchListings, loadListings } from './SearchPage.ducks'; import css from './SearchPage.css'; import { FilterPanel, @@ -14,44 +14,16 @@ import { SearchResultsPanel, } from '../../components'; -const fakeListings = [ - { - id: 123, - title: 'Banyan Studios', - price: '55\u20AC / day', - description: 'Organic Music Production in a Sustainable, Ethical and Professional Studio.', - location: 'New York, NY \u2022 40mi away', - review: { count: '8 reviews', rating: '4' }, - author: { - name: 'The Stardust Collective', - avatar: 'http://placehold.it/44x44', - review: { rating: '4' }, - }, - }, - { - id: 1234, - title: 'Pienix Studio', - price: '80\u20AC day', - description: 'Pienix Studio specializes in music mixing and mastering production.', - location: 'New York, NY \u2022 6mi away', - review: { count: '7 reviews', rating: '4' }, - author: { name: 'Juhan', avatar: 'http://placehold.it/44x44', review: { rating: '4' } }, - }, -]; - export class SearchPageComponent extends Component { - - componentWillMount() { - /* eslint-disable no-console */ - console.log('Client loads data'); - SearchPageComponent.loadData().then((val) => { - console.log('Client fetched data', val); - }); - /* eslint-enable no-console */ + componentDidMount() { + // TODO: This should be moved to Router + this.props.onLoadListings(); } render() { - const { tab } = this.props; + const { tab, SearchPage } = this.props; + const { listings } = SearchPage; + const fakeListings = listings || []; const selectedTab = includes(['filters','listings', 'map'], tab) ? tab : 'listings'; @@ -86,23 +58,25 @@ export class SearchPageComponent extends Component { ); } +} + +SearchPageComponent.loadData = { fetchData: callFetchListings, fetchDataAction: loadListings }; + +SearchPageComponent.defaultProps = { SearchPage: {}, tab: 'listings' }; + +const { array, bool, func, shape, string } = PropTypes; + +SearchPageComponent.propTypes = { + onLoadListings: func.isRequired, + SearchPage: shape({ + filters: array, + initialListingsLoaded: bool, + listings: array, + loadingListings: bool, + }), + tab: string, }; -SearchPageComponent.defaultProps = { tab: 'listings' }; - -const { string } = PropTypes; - -SearchPageComponent.propTypes = { tab: string }; - -SearchPageComponent.loadData = () => ( - new Promise((resolve) => ( - // also node has a 'setTimeout' -> it's good for testing async call. - setTimeout(() => { - resolve(fakeListings); - }, (Math.random() * 2000) + 1000) - )) -); - /** * Container functions. * Since we add this to global store state with combineReducers, this will only get partial state @@ -116,6 +90,7 @@ const mapDispatchToProps = function mapDispatchToProps(dispatch) { return { onAddNotice: msg => dispatch(addFlashNotification('notice', msg)), onAddFilter: (k, v) => dispatch(addFilter(k, v)), + onLoadListings: () => dispatch(loadListings()), }; }; diff --git a/src/containers/SearchPage/SearchPage.test.js b/src/containers/SearchPage/SearchPage.test.js index 5fbb94ea..172db280 100644 --- a/src/containers/SearchPage/SearchPage.test.js +++ b/src/containers/SearchPage/SearchPage.test.js @@ -2,7 +2,7 @@ import React from 'react'; import renderer from 'react-test-renderer'; import { TestProvider } from '../../util/test-helpers'; import { SearchPageComponent } from './SearchPage'; -import reducer, { ADD_FILTER, addFilter } from './SearchPage.ducks'; +import reducer, { ADD_FILTER, addFilter, initialState } from './SearchPage.ducks'; import { RoutesProvider } from '../../components'; import routesConfiguration from '../../routesConfiguration'; @@ -11,7 +11,7 @@ describe('SearchPageComponent', () => { const component = renderer.create( - + v} /> , ); @@ -31,7 +31,7 @@ describe('SearchPageDucs', () => { describe('reducer', () => { it('should return the initial state', () => { const initial = reducer(undefined, {}); - expect(initial).toEqual({}); + expect(initial).toEqual(initialState); }); it('should handle ADD_FILTER', () => { diff --git a/src/containers/SearchPage/__snapshots__/SearchPage.test.js.snap b/src/containers/SearchPage/__snapshots__/SearchPage.test.js.snap index fe5450ef..1b45bcd7 100644 --- a/src/containers/SearchPage/__snapshots__/SearchPage.test.js.snap +++ b/src/containers/SearchPage/__snapshots__/SearchPage.test.js.snap @@ -155,174 +155,6 @@ exports[`SearchPageComponent matches snapshot 1`] = ` ▼
-
-
-
- Listing Title -
-
-
-
- - Banyan Studios - -
- 55€ / day -
-
-
- Organic Music Production in a Sustainable, Ethical and Professional Studio. -
-
-
- New York, NY • 40mi away -
-
- ( - - 4 - - - /5 - - ) - - - 8 reviews - -
-
-
-
-
- The Stardust Collective -
-
- - The Stardust Collective - -
- review: - - 4 - - - /5 - -
-
-
-
-
-
-
-
- Listing Title -
-
-
-
- - Pienix Studio - -
- 80€ day -
-
-
- Pienix Studio specializes in music mixing and mastering production. -
-
-
- New York, NY • 6mi away -
-
- ( - - 4 - - - /5 - - ) - - - 7 reviews - -
-
-
-
-
- Juhan -
-
- - Juhan - -
- review: - - 4 - - - /5 - -
-
-
-
-
-
-
-
- Listing Title -
-
-
- - Banyan Studios - -
- ( - - 4 - - - /5 - - ) - - - 8 reviews - -
-
- 55€ / day -
-
-
-
-
-
- Listing Title -
-
-
- - Pienix Studio - -
- ( - - 4 - - - /5 - - ) - - - 7 reviews - -
-
- 80€ day -
-
-
-
+ className={undefined} /> , document.getElementById('root')); } @@ -31,4 +33,4 @@ if (typeof window !== 'undefined') { // Export the function for server side rendering. export default renderApp; -export { matchLocation }; +export { matchPathname, configureStore }; diff --git a/src/routesConfiguration.js b/src/routesConfiguration.js index 0c1367a6..16112dd8 100644 --- a/src/routesConfiguration.js +++ b/src/routesConfiguration.js @@ -3,7 +3,7 @@ import { find } from 'lodash'; import { Redirect } from 'react-router'; // This will change to `matchPath` soonish -import matchPattern from 'react-router/matchPattern' +import matchPattern from 'react-router/matchPattern'; import pathToRegexp from 'path-to-regexp'; import { @@ -37,24 +37,28 @@ const routesConfiguration = [ exactly: true, name: 'SearchPage', component: SearchPage, + loadData: SearchPage.loadData, routes: [ { pattern: '/s/filters', exactly: true, name: 'SearchFiltersPage', component: props => , + loadData: SearchPage.loadData, }, { pattern: '/s/listings', exactly: true, name: 'SearchListingsPage', component: props => , + loadData: SearchPage.loadData, }, { pattern: '/s/map', exactly: true, name: 'SearchMapPage', component: props => , + loadData: SearchPage.loadData, }, ], }, @@ -276,9 +280,9 @@ const pathByRouteName = (nameToFind, routes, params = {}) => /** * matchRoutesToLocation helps to figure out which routes are related to given location. */ -const matchRoutesToLocation = (routes, location, matchedRoutes=[], params={}) => { +const matchRoutesToLocation = (routes, location, matchedRoutes = [], params = {}) => { const parameters = { ...params }; - routes.forEach((route) => { + routes.forEach(route => { const { exactly = false } = route; const match = !route.pattern ? true : matchPattern(route.pattern, location, exactly); @@ -286,7 +290,9 @@ const matchRoutesToLocation = (routes, location, matchedRoutes=[], params={}) => matchedRoutes.push(route); if (match.params) { - Object.keys(match.params).forEach(key => { parameters[key] = match.params[key]; }); + Object.keys(match.params).forEach(key => { + parameters[key] = match.params[key]; + }); } } @@ -296,12 +302,11 @@ const matchRoutesToLocation = (routes, location, matchedRoutes=[], params={}) => }); return { matchedRoutes, params: parameters }; -} +}; -const matchPathnameCreator = routes => ( - (location, matchedRoutes=[], params={}) => - matchRoutesToLocation(routes, { pathname: location }, matchedRoutes, params) -); +const matchPathnameCreator = routes => + (location, matchedRoutes = [], params = {}) => + matchRoutesToLocation(routes, { pathname: location }, matchedRoutes, params); /** * matchRoutesToLocation helps to figure out which routes are related to given location. diff --git a/src/sagas.js b/src/sagas.js index ae72fac3..d0a86084 100644 --- a/src/sagas.js +++ b/src/sagas.js @@ -1,5 +1,6 @@ import { watchLogin, watchLogout } from './ducks/Auth.ducks'; +import { watchLoadListings } from './containers/SearchPage/SearchPage.ducks'; export default function* rootSaga() { - yield [watchLogin(), watchLogout()]; + yield [watchLogin(), watchLogout(), watchLoadListings()]; } diff --git a/src/store.js b/src/store.js index ad4f25f8..405d6139 100644 --- a/src/store.js +++ b/src/store.js @@ -1,8 +1,7 @@ /* eslint-disable no-underscore-dangle */ import { createStore, applyMiddleware, compose } from 'redux'; -import createSagaMiddleware from 'redux-saga'; +import createSagaMiddleware, { END } from 'redux-saga'; import createReducer from './reducers'; -import rootSaga from './sagas'; /** * Create a new store with the given initial state. Adds Redux @@ -21,9 +20,10 @@ export default function configureStore(initialState = {}) { : compose; const enhancer = composeEnhancers(applyMiddleware(...middlewares)); - const store = createStore(createReducer(), initialState, enhancer); - sagaMiddleware.run(rootSaga); + const store = createStore(createReducer(), initialState, enhancer); + store.runSaga = sagaMiddleware.run; + store.close = () => store.dispatch(END); return store; } From b937f3fd57fc17de5563f7d57aea6c580bc71e83 Mon Sep 17 00:00:00 2001 From: Vesa Luusua Date: Fri, 3 Feb 2017 15:51:01 +0200 Subject: [PATCH 4/6] Refactor saga code in ducks files and add sdk as parameter. --- .../fakeData.js => server/fakeSDK.js | 20 +++++- server/index.js | 11 +++- src/containers/SearchPage/SearchPage.ducks.js | 65 ++++++++++--------- src/containers/SearchPage/SearchPage.js | 7 +- src/sagas.js | 3 +- src/util/fakeSDK.js | 45 +++++++++++++ src/util/sagaHelpers.js | 14 ++++ 7 files changed, 127 insertions(+), 38 deletions(-) rename src/containers/SearchPage/fakeData.js => server/fakeSDK.js (62%) create mode 100644 src/util/fakeSDK.js create mode 100644 src/util/sagaHelpers.js diff --git a/src/containers/SearchPage/fakeData.js b/server/fakeSDK.js similarity index 62% rename from src/containers/SearchPage/fakeData.js rename to server/fakeSDK.js index a543d6f6..b06c6371 100644 --- a/src/containers/SearchPage/fakeData.js +++ b/server/fakeSDK.js @@ -1,5 +1,5 @@ -/* eslint-disable import/prefer-default-export */ -export const fakeListings = [ +/* eslint-disable import/prefer-default-export, no-unused-vars */ +const fakeListings = [ { id: 123, title: 'Banyan Studios', @@ -24,3 +24,19 @@ export const fakeListings = [ }, ]; /* eslint-enable import/prefer-default-export */ + +exports.fetchListings = () => { + const randomTime = Math.random() * 2000; + const fakeResponseTime = 1000 + randomTime; + + const fakeFetch = new Promise((resolve, reject) => { + setTimeout( + () => { + resolve({ response: fakeListings }); + // or reject({ error: new Error('FetchListings errored') ); + }, + fakeResponseTime, + ); + }); + return fakeFetch; +}; diff --git a/server/index.js b/server/index.js index 944d5920..abc391bb 100644 --- a/server/index.js +++ b/server/index.js @@ -26,8 +26,9 @@ const url = require('url'); const _ = require('lodash'); const React = require('react'); const { createServerRenderContext } = require('react-router'); -const auth = require('./auth'); const sagaEffects = require('redux-saga/effects'); +const auth = require('./auth'); +const sdk = require('./fakeSDK'); // Construct the bundle path where the server side rendering function // can be imported. @@ -74,16 +75,21 @@ const template = _.template(indexHtml, { function fetchInitialState(requestUrl) { const pathname = url.parse(requestUrl).pathname; const { matchedRoutes, params } = matchPathname(pathname); + + // pathname may match with several routes (if they don't have exactly=true) + // We filter all the components form matched routes that have `loadData` const initialFetches = _ .chain(matchedRoutes) .filter(r => r.loadData && r.loadData.fetchData) - .map(r => sagaEffects.fork(r.loadData.fetchData)) + .map(r => sagaEffects.fork(r.loadData.fetchData, sdk)) .value(); + // We need to combine different onload sagas under one yield const fetchInitialData = function* fetchInitialData() { yield initialFetches; }; + // runSaga (if necessary) and return initial store state after loadData fetches. if (initialFetches.length > 0) { const fetchPromise = new Promise((resolve, reject) => { const store = configureStore({}); @@ -94,6 +100,7 @@ function fetchInitialState(requestUrl) { .catch(e => { reject(e); }); + // Close temporary store (which was used only for fetching initial store state) store.close(); }); return fetchPromise; diff --git a/src/containers/SearchPage/SearchPage.ducks.js b/src/containers/SearchPage/SearchPage.ducks.js index 9f4bfa8d..810f90f7 100644 --- a/src/containers/SearchPage/SearchPage.ducks.js +++ b/src/containers/SearchPage/SearchPage.ducks.js @@ -4,16 +4,15 @@ * https://github.com/erikras/ducks-modular-redux */ import { unionWith, isEqual } from 'lodash'; -import { delay } from 'redux-saga'; import { call, take, put } from 'redux-saga/effects'; -import { fakeListings } from './fakeData'; +import { createRequestTypes } from '../../util/sagaHelpers'; // ================ Action types ================ // export const ADD_FILTER = 'app/SearchPage/ADD_FILTER'; -export const LOAD_LISTINGS = 'app/SearchPage/LOAD_LISTINGS'; -export const LOAD_LISTINGS_SUCCESS = 'app/SearchPage/LOAD_LISTINGS_SUCCESS'; -export const LOAD_LISTINGS_FAILURE = 'app/SearchPage/LOAD_LISTINGS_FAILURE'; + +// async actions use request - success / failure pattern +export const LOAD_LISTINGS = createRequestTypes('app/SearchPage/LOAD_LISTINGS'); // ================ Reducer ================ // @@ -26,17 +25,16 @@ export const initialState = { export default function reducer(state = initialState, action = {}) { const { type, payload } = action; + switch (type) { case ADD_FILTER: { const stateFilters = state.filters || []; return { ...state, ...{ filters: unionWith(stateFilters, [payload], isEqual) } }; } - case LOAD_LISTINGS: - return { ...state, loadingListings: true, initialListingsLoaded: false }; - case LOAD_LISTINGS_SUCCESS: - return { ...state, loadingListings: false, initialListingsLoaded: true, listings: payload }; - case LOAD_LISTINGS_FAILURE: - return { ...state, loadingListings: false, loadListingError: payload }; + case LOAD_LISTINGS.REQUEST: + case LOAD_LISTINGS.SUCCESS: + case LOAD_LISTINGS.FAILURE: + return { ...state, ...payload }; default: return state; } @@ -45,37 +43,42 @@ export default function reducer(state = initialState, action = {}) { // ================ Action creators ================ // export const addFilter = (key, value) => ({ type: ADD_FILTER, payload: { [key]: value } }); -export const loadListings = () => ({ type: LOAD_LISTINGS, payload: {} }); -export const loadListingsSuccess = listings => ({ type: LOAD_LISTINGS_SUCCESS, payload: listings }); -export const loadListingsFailure = error => ({ - type: LOAD_LISTINGS_FAILURE, - payload: error, - error: true, -}); + +// async actions use request - success - failure pattern +const action = (type, payload = {}) => ({ type, ...payload }); + +export const loadListings = { + request: () => + action(LOAD_LISTINGS.REQUEST, { + payload: { loadingListings: true, initialListingsLoaded: false }, + }), + success: listings => + action(LOAD_LISTINGS.SUCCESS, { + payload: { loadingListings: false, initialListingsLoaded: true, listings }, + }), + failure: error => + action(LOAD_LISTINGS.FAILURE, { + payload: { loadingListings: false, loadListingError: error, error: true }, + }), +}; // ================ Worker sagas ================ // -export function fetchListings() { - const randomTime = Math.random() * 2000; - const fakeResponseTime = 1000 + randomTime; - return delay(fakeResponseTime, { response: fakeListings }); -} - -export function* callFetchListings() { - const { response, error } = yield call(fetchListings); +export function* callFetchListings(sdk) { + const { response, error } = yield call(sdk.fetchListings); if (response) { - yield put(loadListingsSuccess(response)); + yield put(loadListings.success(response)); } else { - yield put(loadListingsFailure(error)); + yield put(loadListings.failure(error)); } } // ================ Watcher sagas ================ // -export function* watchLoadListings() { +export function* watchLoadListings(sdk) { // eslint-disable-next-line no-constant-condition while (true) { - yield take(LOAD_LISTINGS); - yield call(callFetchListings); + yield take(LOAD_LISTINGS.REQUEST); + yield call(callFetchListings, sdk); } } diff --git a/src/containers/SearchPage/SearchPage.js b/src/containers/SearchPage/SearchPage.js index 590e58f5..7dffcfec 100644 --- a/src/containers/SearchPage/SearchPage.js +++ b/src/containers/SearchPage/SearchPage.js @@ -17,7 +17,10 @@ import { export class SearchPageComponent extends Component { componentDidMount() { // TODO: This should be moved to Router - this.props.onLoadListings(); + const { SearchPage } = this.props; + if (!SearchPage.initialListingsLoaded) { + this.props.onLoadListings(); + } } render() { @@ -90,7 +93,7 @@ const mapDispatchToProps = function mapDispatchToProps(dispatch) { return { onAddNotice: msg => dispatch(addFlashNotification('notice', msg)), onAddFilter: (k, v) => dispatch(addFilter(k, v)), - onLoadListings: () => dispatch(loadListings()), + onLoadListings: () => dispatch(loadListings.request()), }; }; diff --git a/src/sagas.js b/src/sagas.js index d0a86084..f1df18e3 100644 --- a/src/sagas.js +++ b/src/sagas.js @@ -1,6 +1,7 @@ import { watchLogin, watchLogout } from './ducks/Auth.ducks'; import { watchLoadListings } from './containers/SearchPage/SearchPage.ducks'; +import sdk from './util/fakeSDK'; export default function* rootSaga() { - yield [watchLogin(), watchLogout(), watchLoadListings()]; + yield [watchLogin(), watchLogout(), watchLoadListings(sdk)]; } diff --git a/src/util/fakeSDK.js b/src/util/fakeSDK.js new file mode 100644 index 00000000..5affbb92 --- /dev/null +++ b/src/util/fakeSDK.js @@ -0,0 +1,45 @@ +/* eslint-disable import/prefer-default-export, no-unused-vars */ +export const fakeListings = [ + { + id: 123, + title: 'Banyan Studios', + price: '55\u20AC / day', + description: 'Organic Music Production in a Sustainable, Ethical and Professional Studio.', + location: 'New York, NY \u2022 40mi away', + review: { count: '8 reviews', rating: '4' }, + author: { + name: 'The Stardust Collective', + avatar: 'http://placehold.it/44x44', + review: { rating: '4' }, + }, + }, + { + id: 1234, + title: 'Pienix Studio', + price: '80\u20AC / day', + description: 'Pienix Studio specializes in music mixing and mastering production.', + location: 'New York, NY \u2022 6mi away', + review: { count: '7 reviews', rating: '4' }, + author: { name: 'Juhan', avatar: 'http://placehold.it/44x44', review: { rating: '4' } }, + }, +]; +/* eslint-enable import/prefer-default-export */ + +const fakeSDK = { + fetchListings: () => { + const randomTime = Math.random() * 2000; + const fakeResponseTime = 1000 + randomTime; + + const fakeFetch = new Promise((resolve, reject) => { + setTimeout( + () => { + resolve({ response: fakeListings }); + // or reject({ error: new Error('FetchListings errored') ); + }, + fakeResponseTime, + ); + }); + return fakeFetch; + }, +}; +export default fakeSDK; diff --git a/src/util/sagaHelpers.js b/src/util/sagaHelpers.js new file mode 100644 index 00000000..099ad65a --- /dev/null +++ b/src/util/sagaHelpers.js @@ -0,0 +1,14 @@ +/* eslint-disable import/prefer-default-export */ + +// async actions use request - success / failure pattern +const REQUEST = 'REQUEST'; +const SUCCESS = 'SUCCESS'; +const FAILURE = 'FAILURE'; + +// response format: { REQUEST: 'baseStr.REQUEST', SUCCESS: 'baseStr.SUCCESS', FAILURE: 'baseStr.FAILURE' } +export const createRequestTypes = base => [REQUEST, SUCCESS, FAILURE].reduce((acc, type) => { + // eslint-disable-next-line no-param-reassign + acc[type] = `${base}.${type}`; + return acc; +}, {}); +/* eslint-enable import/prefer-default-export */ From f508886d9f6fd4b77717da227a044e5cdc36359f Mon Sep 17 00:00:00 2001 From: Vesa Luusua Date: Fri, 3 Feb 2017 00:11:20 +0200 Subject: [PATCH 5/6] Strange bug: tests don't work without this fix. Will be changed later. --- src/routesConfiguration.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/routesConfiguration.js b/src/routesConfiguration.js index 16112dd8..39c26f44 100644 --- a/src/routesConfiguration.js +++ b/src/routesConfiguration.js @@ -37,28 +37,28 @@ const routesConfiguration = [ exactly: true, name: 'SearchPage', component: SearchPage, - loadData: SearchPage.loadData, + loadData: SearchPage ? SearchPage.loadData : null, routes: [ { pattern: '/s/filters', exactly: true, name: 'SearchFiltersPage', component: props => , - loadData: SearchPage.loadData, + loadData: SearchPage ? SearchPage.loadData : null, }, { pattern: '/s/listings', exactly: true, name: 'SearchListingsPage', component: props => , - loadData: SearchPage.loadData, + loadData: SearchPage ? SearchPage.loadData : null, }, { pattern: '/s/map', exactly: true, name: 'SearchMapPage', component: props => , - loadData: SearchPage.loadData, + loadData: SearchPage ? SearchPage.loadData : null, }, ], }, From bc2baaf3b5b3abde9e0e7782ae4b9b9eb3e4265f Mon Sep 17 00:00:00 2001 From: Vesa Luusua Date: Tue, 7 Feb 2017 13:13:08 +0200 Subject: [PATCH 6/6] Refactoring to improve readability, compatibility with saga and comments --- server/fakeSDK.js | 11 ++--------- server/index.js | 8 ++++---- src/containers/SearchPage/SearchPage.ducks.js | 13 +++++-------- src/containers/SearchPage/SearchPage.js | 6 +++--- src/index.js | 3 +++ src/routesConfiguration.js | 16 +++++++++------- src/store.js | 2 +- src/util/fakeSDK.js | 12 +++--------- 8 files changed, 30 insertions(+), 41 deletions(-) diff --git a/server/fakeSDK.js b/server/fakeSDK.js index b06c6371..86effa36 100644 --- a/server/fakeSDK.js +++ b/server/fakeSDK.js @@ -29,14 +29,7 @@ exports.fetchListings = () => { const randomTime = Math.random() * 2000; const fakeResponseTime = 1000 + randomTime; - const fakeFetch = new Promise((resolve, reject) => { - setTimeout( - () => { - resolve({ response: fakeListings }); - // or reject({ error: new Error('FetchListings errored') ); - }, - fakeResponseTime, - ); + return new Promise((resolve, reject) => { + setTimeout(() => resolve(fakeListings), fakeResponseTime); }); - return fakeFetch; }; diff --git a/server/index.js b/server/index.js index abc391bb..9670ce53 100644 --- a/server/index.js +++ b/server/index.js @@ -80,8 +80,8 @@ function fetchInitialState(requestUrl) { // We filter all the components form matched routes that have `loadData` const initialFetches = _ .chain(matchedRoutes) - .filter(r => r.loadData && r.loadData.fetchData) - .map(r => sagaEffects.fork(r.loadData.fetchData, sdk)) + .filter(r => r.loadData) + .map(r => sagaEffects.fork(r.loadData, sdk)) .value(); // We need to combine different onload sagas under one yield @@ -100,8 +100,8 @@ function fetchInitialState(requestUrl) { .catch(e => { reject(e); }); - // Close temporary store (which was used only for fetching initial store state) - store.close(); + // Close temporary store's saga middleware (which was used only for fetching initial store state) + store.closeSagaMiddleware(); }); return fetchPromise; } diff --git a/src/containers/SearchPage/SearchPage.ducks.js b/src/containers/SearchPage/SearchPage.ducks.js index 810f90f7..68626377 100644 --- a/src/containers/SearchPage/SearchPage.ducks.js +++ b/src/containers/SearchPage/SearchPage.ducks.js @@ -4,7 +4,7 @@ * https://github.com/erikras/ducks-modular-redux */ import { unionWith, isEqual } from 'lodash'; -import { call, take, put } from 'redux-saga/effects'; +import { call, put, takeEvery } from 'redux-saga/effects'; import { createRequestTypes } from '../../util/sagaHelpers'; // ================ Action types ================ // @@ -65,10 +65,10 @@ export const loadListings = { // ================ Worker sagas ================ // export function* callFetchListings(sdk) { - const { response, error } = yield call(sdk.fetchListings); - if (response) { + try { + const response = yield call(sdk.fetchListings); yield put(loadListings.success(response)); - } else { + } catch(error) { yield put(loadListings.failure(error)); } } @@ -77,8 +77,5 @@ export function* callFetchListings(sdk) { export function* watchLoadListings(sdk) { // eslint-disable-next-line no-constant-condition - while (true) { - yield take(LOAD_LISTINGS.REQUEST); - yield call(callFetchListings, sdk); - } + yield takeEvery(LOAD_LISTINGS.REQUEST, callFetchListings, sdk); } diff --git a/src/containers/SearchPage/SearchPage.js b/src/containers/SearchPage/SearchPage.js index 7dffcfec..1b48c7fd 100644 --- a/src/containers/SearchPage/SearchPage.js +++ b/src/containers/SearchPage/SearchPage.js @@ -34,10 +34,10 @@ export class SearchPageComponent extends Component { const filtersClassName = classNames(css.filters, { [css.open]: selectedTab === 'filters', }); - const listingsClassName = classNames(css.filters, { + const listingsClassName = classNames(css.listings, { [css.open]: selectedTab === 'listings', }); - const mapClassName = classNames(css.filters, { + const mapClassName = classNames(css.map, { [css.open]: selectedTab === 'map', }); @@ -63,7 +63,7 @@ export class SearchPageComponent extends Component { } } -SearchPageComponent.loadData = { fetchData: callFetchListings, fetchDataAction: loadListings }; +SearchPageComponent.loadData = callFetchListings; SearchPageComponent.defaultProps = { SearchPage: {}, tab: 'listings' }; diff --git a/src/index.js b/src/index.js index 5e9f4bf5..8f61618f 100644 --- a/src/index.js +++ b/src/index.js @@ -33,4 +33,7 @@ if (typeof window !== 'undefined') { // Export the function for server side rendering. export default renderApp; +// exporting matchPathname and configureStore for server side rendering. +// matchPathname helps to figure out which route is called and if it has preloading needs +// configureStore is used for creating initial store state for Redux after preloading export { matchPathname, configureStore }; diff --git a/src/routesConfiguration.js b/src/routesConfiguration.js index 39c26f44..09df1016 100644 --- a/src/routesConfiguration.js +++ b/src/routesConfiguration.js @@ -281,27 +281,29 @@ const pathByRouteName = (nameToFind, routes, params = {}) => * matchRoutesToLocation helps to figure out which routes are related to given location. */ const matchRoutesToLocation = (routes, location, matchedRoutes = [], params = {}) => { - const parameters = { ...params }; + let parameters = { ...params }; + let matched = [...matchedRoutes]; + routes.forEach(route => { const { exactly = false } = route; const match = !route.pattern ? true : matchPattern(route.pattern, location, exactly); if (match) { - matchedRoutes.push(route); + matched.push(route); if (match.params) { - Object.keys(match.params).forEach(key => { - parameters[key] = match.params[key]; - }); + parameters = { ...parameters, ...match.params }; } } if (route.routes) { - matchRoutesToLocation(route.routes, location, matchedRoutes, parameters); + const { matchedRoutes: subRouteMatches, params: subRouteParams } = matchRoutesToLocation(route.routes, location, matched, parameters); + matched = matched.concat(subRouteMatches); + parameters = { ...parameters, ...subRouteParams }; } }); - return { matchedRoutes, params: parameters }; + return { matchedRoutes: matched, params: parameters }; }; const matchPathnameCreator = routes => diff --git a/src/store.js b/src/store.js index 405d6139..a431d492 100644 --- a/src/store.js +++ b/src/store.js @@ -23,7 +23,7 @@ export default function configureStore(initialState = {}) { const store = createStore(createReducer(), initialState, enhancer); store.runSaga = sagaMiddleware.run; - store.close = () => store.dispatch(END); + store.closeSagaMiddleware = () => store.dispatch(END); return store; } diff --git a/src/util/fakeSDK.js b/src/util/fakeSDK.js index 5affbb92..c298a311 100644 --- a/src/util/fakeSDK.js +++ b/src/util/fakeSDK.js @@ -30,16 +30,10 @@ const fakeSDK = { const randomTime = Math.random() * 2000; const fakeResponseTime = 1000 + randomTime; - const fakeFetch = new Promise((resolve, reject) => { - setTimeout( - () => { - resolve({ response: fakeListings }); - // or reject({ error: new Error('FetchListings errored') ); - }, - fakeResponseTime, - ); + return new Promise((resolve, reject) => { + setTimeout(() => resolve(fakeListings), fakeResponseTime); }); - return fakeFetch; }, }; + export default fakeSDK;