BRS 7 - Full SSR Hard Source and Loadable support #77
Conversation
* (Temporarily) added hard-source plugin * Write loadable-stats to disk for consumption by server
…isation, definePlugin typeof window
Fix iconv loader
| @@ -55,6 +57,10 @@ const shouldUseSourceMap = process.env.GENERATE_SOURCEMAP !== 'false'; | |||
| // Some apps do not need the benefits of saving a web request, so not inlining the chunk | |||
| // makes for a smoother build process. | |||
| const shouldInlineRuntimeChunk = process.env.INLINE_RUNTIME_CHUNK !== 'false'; | |||
| // We might not want to use the hard source plugin on environments that won't persist the cache for later | |||
| const useHardSourceWebpackPlugin = | |||
There was a problem hiding this comment.
Rather than an env variable this might be better as a config value instead?
There was a problem hiding this comment.
As it's experimental (mainly because mzgoddard/hard-source-webpack-plugin#419 + waiting for Webpack 5), I've found value in flipping it on/off at will - so was thinking the environment variables maybe gave more flexibility.
If there's cleaner way without needing to make a code change in the consuming project that would still be good.
| @@ -54,6 +56,10 @@ const shouldUseSourceMap = process.env.GENERATE_SOURCEMAP !== 'false'; | |||
| // Some apps do not need the benefits of saving a web request, so not inlining the chunk | |||
| // makes for a smoother build process. | |||
| // const shouldInlineRuntimeChunk = process.env.INLINE_RUNTIME_CHUNK !== 'false'; | |||
| // We might not want to use the hard source plugin on environments that won't persist the cache for later | |||
| const useHardSourceWebpackPlugin = | |||
| @@ -82,7 +88,7 @@ module.exports = function(webpackEnv) { | |||
| : isEnvDevelopment && '/'; | |||
| // Some apps do not use client-side routing with pushState. | |||
| // For these, "homepage" can be set to "." to enable relative asset paths. | |||
| const shouldUseRelativeAssetPaths = publicPath === './'; | |||
| // const shouldUseRelativeAssetPaths = publicPath === './'; | |||
There was a problem hiding this comment.
Could you confirm why this has been removed?
There was a problem hiding this comment.
It was only used by MiniCssExtractPlugin
| shouldUseRelativeAssetPaths ? { publicPath: '../../' } : undefined | ||
| ), | ||
| }, | ||
| // isEnvDevelopment && require.resolve('style-loader'), |
There was a problem hiding this comment.
Could you confirm why this has been removed?
There was a problem hiding this comment.
In standard CRA, in development, styles are served client-side only, via the Webpack dev server. The CSS is served inside the .js assets.
I didn't want to get into changing that - so in development, SSR should just ignore styles completely.
| new webpack.DefinePlugin(env.stringified), | ||
| new webpack.DefinePlugin({ | ||
| ...env.stringified, | ||
| 'typeof window': '"undefined"', |
There was a problem hiding this comment.
Could you confirm what this is doing?
There was a problem hiding this comment.
https://webpack.js.org/plugins/define-plugin/#usage - defining typeof window is the closest I've seen to a 'standard' way of stripping out client/server-only code from their respective bundles.
| // This is necessary to emit hot updates (currently CSS only): | ||
| isEnvDevelopment && new webpack.HotModuleReplacementPlugin(), | ||
| // isEnvDevelopment && new webpack.HotModuleReplacementPlugin(), |
There was a problem hiding this comment.
Could you confirm why this has been removed?
There was a problem hiding this comment.
As above re: styles, and I don't think https://webpack.js.org/concepts/hot-module-replacement/ is designed for the server.
| filename: 'ssr.css', | ||
| // chunkFilename: 'static/css/[name].[contenthash:8].chunk.css', | ||
| }), | ||
| // isEnvProduction && |
There was a problem hiding this comment.
Could you confirm why this has been removed? Guessing it relates to the HardSourcePlugin?
Adding new ignore option
olliecurtis
changed the title
olliecurtis
changed the title
|
Closing this PR as v7 is no longer supported version at Skyscanner - these changes have been ported to newer versions and also improved in #95 |
See these PRs for full description: