Implement code splitting with react-loadable #84
Conversation
src/components/html/Html.js
Outdated
| @@ -1,7 +1,7 @@ | |||
| import React, { PropTypes } from 'react'; | |||
| import serialize from 'serialize-javascript'; | |||
|
|
|||
| function Html({ css, js, html, head, initialState }) { | |||
| function Html({ css, mainJs, chunkJs, html, head, initialState }) { | |||
There was a problem hiding this comment.
Merge mainJs and chunkJs as single js prop.
There was a problem hiding this comment.
(and include bootstrap)
| // (without fetchData...) if it's moved up then we don't split any redux selectors / actions. | ||
| // - Perhaps it's worth digging up that redux-saga SSR depenency solution instead so we rely | ||
| // less on components. | ||
| // - Or perhaps I can wrap think middleware to provide an api to identifiy what data is needed? |
There was a problem hiding this comment.
TODO: Delete comment
| // dispatch `fetchQuoteIfNeeded` when the LoadingComponent mounts to start fetching | ||
| // data deps earlier, or maybe even introduce another static method e.g. 'preFetch' | ||
| // which could just make the api call and have Index dispatch it... Worth benchmarking. | ||
| export default liftFetchData(webpackRequireWeakId)(IndexLoadable); |
There was a problem hiding this comment.
const enhance = compose(liftFetchData(webpackRequireWeakId), loadable);
export default enhance({
loader: () => import('./Index'),
LoadingComponent,
webpackRequireWeakId
});
| @@ -0,0 +1,14 @@ | |||
| /** | |||
| * Lifts static `fetchData` method to `Component` to allow server to fetch | |||
| * data without having to import Index. | |||
There was a problem hiding this comment.
*Lifts static fetchData method from weakId component to Component, deliberately circumventing static require / import to prevent Webpack from bundling the weakId component and it's deps.
| return class extends Component { | ||
| constructor() { | ||
| super(); | ||
| moduleIds.push(options.webpackRequireWeakId()); |
There was a problem hiding this comment.
What happens if this is used twice, perhaps moduleIds should be a unique Set?
server/routes/static.js
Outdated
| try { | ||
| serverStats = require(SERVER_STATS_PATH); | ||
| } catch (ex) { | ||
| throw new Error('Client bundle server-stats.json not found. Try running `npm run build`'); |
There was a problem hiding this comment.
Fixup error message.
|
|
||
| // NOTE: We're making a trade off for more aggresive code splitting (i.e. includes | ||
| // action creators) for waterfall requests when fetching the chunk and the data | ||
| // in the client. |
There was a problem hiding this comment.
Try splitting reducer.
There was a problem hiding this comment.
src/reducers/quote/quote.js
Outdated
| // FETCH_QUOTE_REQUEST, | ||
| // FETCH_QUOTE_SUCCESS, | ||
| // FETCH_QUOTE_FAILURE | ||
| // } from 'actions/quote/quote'; |
src/reducers/quote/quote.js
Outdated
| return { | ||
| ...state, | ||
| isFetching: false, | ||
| value: action.payload | ||
| }; | ||
| case FETCH_QUOTE_FAILURE: | ||
| case 'FETCH_QUOTE_FAILURE': |
There was a problem hiding this comment.
Don't use strings
src/server.js
Outdated
| function getJs(stats, moduleIds) { | ||
| return [ | ||
| getJsByChunkName('bootstrap', stats), | ||
| ...getCodeSplitChunks(stats, moduleIds), |
There was a problem hiding this comment.
Rename to getCodeSplitJs
tasks/build/createConfig/index.js
Outdated
| {} | ||
| ) | ||
| : {}, | ||
| // Not sure there's much point in moving node_modules to externals but |
There was a problem hiding this comment.
Sort out why removing externals causes:
WARNING Compiled with 1 warnings 10:57:15 PM
warning in ./~/encoding/lib/iconv-loader.js
9:12-34 Critical dependency: the request of a dependency is an expression
| /** | ||
| * Move webpack bootstrap to separate chunk to allow code split chunks to be | ||
| * loaded *before* main. | ||
| */ |
There was a problem hiding this comment.
Not sure this comment is necessary...it's already documented in createConfig/index.js
| }) | ||
| }), | ||
| // Deterministic module ids for long term caching | ||
| new webpack.HashedModuleIdsPlugin() |
There was a problem hiding this comment.
Adds ~10kb to total bundle size vs default indexed module ids but prob fine until I can look into webpack records.
| @@ -2,10 +2,10 @@ | |||
|
|
|||
| const StatsWebpackPlugin = require('stats-webpack-plugin'); | |||
|
|
|||
| module.exports = ({ stats }) => { | |||
| module.exports = ({ stats, name }) => { | |||
There was a problem hiding this comment.
make ternary like bootstrapChunk
Approach:
rewind) from wrapped react-loadable instead of absolute module paths as Webpack makes it hard to talk in absolute paths (and of course, both client and server renderers are bundled with webpack).TODO:
See review comments.
Style error and loader components (will do in another PR)Use functions expressions (with arrows) over function declarations and update prettier to use 2 spaces. (prob worth doing in another PR after this is merged)