timdream
commented
6 years ago So I spend some time figuring out how Calypso is built and how it asks Webpack split the chunks. Thankfully there is npm run analyze-bundles to visualize the chunk size and content.
Essentially, this issue asks for the ability to reduce this part of the build.js bundle:
There are three targets — reducers, data-layers, and selectors. I decided to tackle reducers given prior art exists (1 and 2).
To simplify the experiment, I've selected state/timezones/reducer as the target.
I've come up with two approaches: the first one touches the bundler and has it load the reducers the async bundles asks.
diff --git a/client/devdocs/index.js b/client/devdocs/index.js
index ef26b5bcbe..3245f90f16 100644
--- a/client/devdocs/index.js
+++ b/client/devdocs/index.js
@@ -13,5 +13,9 @@ import config from 'config';
import controller from './controller';
-export default function() {
+import { combineReducers } from 'state/utils';
+import { reducers } from 'state';
+import timezones from 'state/timezones/reducer';
+
+export default function sectionEntry() {
if ( config.isEnabled( 'devdocs' ) ) {
page( '/devdocs', controller.sidebar, controller.devdocs );
@@ -35,2 +39,4 @@ export default function() {
}
}
+
+sectionEntry.reducer = combineReducers( Object.assign( {}, reducers, { timezones } ) );
diff --git a/client/state/index.js b/client/state/index.js
index d23743d11f..686d1b79db 100644
--- a/client/state/index.js
+++ b/client/state/index.js
@@ -73,5 +73,5 @@ import storedCards from './stored-cards/reducer';
import support from './support/reducer';
import terms from './terms/reducer';
-import timezones from './timezones/reducer';
+//import timezones from './timezones/reducer';
import themes from './themes/reducer';
import ui from './ui/reducer';
@@ -89,5 +89,5 @@ import config from 'config';
const extensions = combineReducers( extensionsModule.reducers() );
-const reducers = {
+export const reducers = {
analyticsTracking,
accountRecovery,
@@ -147,5 +147,5 @@ const reducers = {
support,
terms,
- timezones,
+ //timezones,
themes,
ui,
diff --git a/server/bundler/loader.js b/server/bundler/loader.js
index 4d4ad2a5de..9e117bc341 100644
--- a/server/bundler/loader.js
+++ b/server/bundler/loader.js
@@ -119,5 +119,7 @@ function splitTemplate( path, section ) {
' controller.setSection( ' + sectionString + ' )( context );',
' if ( ! _loadedSections[ ' + moduleString + ' ] ) {',
- ' require( ' + moduleString + ' )( controller.clientRouter );',
+ ' var module = require( ' + moduleString + ' );',
+ ' module( controller.clientRouter );',
+ ' module.reducer && context.store.replaceReducer( module.reducer );',
' _loadedSections[ ' + moduleString + ' ] = true;',
' }',This is a fragile setup because we are asking everyone annotate the top level section (i.e. devdocs) to ensure their React componments work correctly. So I decided to implement something saner instead. This diff implements a globally-available injectReducers and use it in QueryTimezones:
diff --git a/client/components/data/query-timezones/index.jsx b/client/components/data/query-timezones/index.jsx
index b3cf286440..0a8ccdae1a 100644
--- a/client/components/data/query-timezones/index.jsx
+++ b/client/components/data/query-timezones/index.jsx
@@ -13,4 +13,8 @@ import { connect } from 'react-redux';
*/
import { requestTimezones } from 'state/timezones/actions';
+import timezonesReducer from 'state/timezones/reducer';
+import { injectReducers } from 'state/reducer-inject';
+
+injectReducers( { timezones: timezonesReducer } );
export class QueryTimezones extends Component {
diff --git a/client/state/index.js b/client/state/index.js
index d23743d11f..07c9846a70 100644
--- a/client/state/index.js
+++ b/client/state/index.js
@@ -62,4 +62,5 @@ import purchases from './purchases/reducer';
import reader from './reader/reducer';
import receipts from './receipts/reducer';
+import { setInitialReducers, reducerInjecter } from './reducer-inject';
import sharing from './sharing/reducer';
import shortcodes from './shortcodes/reducer';
@@ -73,5 +74,5 @@ import storedCards from './stored-cards/reducer';
import support from './support/reducer';
import terms from './terms/reducer';
-import timezones from './timezones/reducer';
+//import timezones from './timezones/reducer';
import themes from './themes/reducer';
import ui from './ui/reducer';
@@ -89,4 +90,6 @@ import config from 'config';
const extensions = combineReducers( extensionsModule.reducers() );
+const placeholderReducer = ( state = {} ) => state;
+
const reducers = {
analyticsTracking,
@@ -147,5 +150,10 @@ const reducers = {
support,
terms,
- timezones,
+ // XXX Improve this without referencing the reducer in the build chunk.
+ timezones: combineReducers( {
+ rawOffsets: placeholderReducer,
+ labels: placeholderReducer,
+ byContinents: placeholderReducer,
+ } ),
themes,
ui,
@@ -157,4 +165,5 @@ const reducers = {
export const reducer = combineReducers( reducers );
+setInitialReducers( reducers );
/**
@@ -201,4 +210,5 @@ export function createReduxStore( initialState = {} ) {
const enhancers = [
isBrowser && window.app && window.app.isDebug && consoleDispatcher,
+ isBrowser && config.isEnabled( 'code-splitting' ) && reducerInjecter,
applyMiddleware( ...middlewares ),
isBrowser && sitesSync,
diff --git a/client/state/reducer-inject/index.js b/client/state/reducer-inject/index.js
new file mode 100644
index 0000000000..920ed00589
--- /dev/null
+++ b/client/state/reducer-inject/index.js
@@ -0,0 +1,44 @@
+/**
+ * Reducer injecter Redux store enhancer
+ *
+ * `reducerInjecter` injects into the `createStore` enhancer chain
+ * in order to provide a globally available method to update the reducer
+ * from newly loaded chunks.
+ */
+
+/**
+ * External dependencies
+ */
+import { forIn } from 'lodash';
+
+/**
+ * Internal dependencies
+ */
+import { combineReducers } from 'state/utils';
+
+let store;
+let currentReducers;
+
+export const setInitialReducers = ( reducers ) => {
+ currentReducers = reducers;
+};
+
+export const injectReducers = ( reducers ) => {
+ let updated = false;
+
+ forIn( reducers, ( reducer, key ) => {
+ if ( currentReducers[ key ] && currentReducers[ key ] === reducer ) {
+ return
+ }
+
+ currentReducers[ key ] = reducer;
+ updated = true;
+ } );
+
+ updated && store.replaceReducer( combineReducers( currentReducers ) );
+};
+
+export const reducerInjecter = next => ( reducer, initialState ) => {
+ store = next( reducer, initialState );
+ return store;
+};state/timezone is being removed nicely; see if you could spot the difference :)
There are some improvements to be made here before this can be pushed as a pull request:
- [ ] Noted the place where
XXXis marked in the second patch though: Without it we will likely throw out the presistent initial state. Something needs to be done to generate these placeholder reducers effectively. - [ ] I am using the CommonJS module context to keep the
storeandcurrentReducersglobally, which is not a good pratice but I am running out of ideas. I don't know if we care. - [ ] We'll need to verify
injectReducerswon't degrade the performance of the app. - [ ] One last important point: It would be tedious to manually move every reducer to the state users, but I have not yet figure out how we could do this programmically. Perhaps pass the files through a special Webpack module rule? Let me know the scope of this project we have in mind.
@ehg @apeatling Let me know what you think, especially on the proposed approach and the scope. Thanks.
ehg
jsnajdr
coderkevin
dmsnell
tyxla
Figure out a way to async-load the pieces of our state engine—reducers/selectors/data-layer—in a way that doesn’t detract from the experience of a global, centralized, and readily available data store. Alternatively, avoid pulling large dependencies on reducers by modularizing further or separating concerns as there is no immediate reason for our state reducers to be a significant drag on code weight.