From e977ce2ece087dd94d306309b87039394f7be587 Mon Sep 17 00:00:00 2001 From: Sean Matheson Date: Wed, 5 Apr 2017 16:30:18 +0100 Subject: [PATCH] Changes process.env build flags to be inline with expected string type of typical process.env member. closes #395 --- client/index.js | 11 ++++------ client/registerServiceWorker.js | 3 ++- config/index.js | 22 +++++++++++-------- config/values.js | 17 +++++--------- internal/webpack/configFactory.js | 16 +++++++++----- server/index.js | 2 +- .../middleware/reactApplication/ServerHTML.js | 4 +++- .../getClientBundleEntryAssets.js | 2 +- server/middleware/reactApplication/index.js | 13 +++++------ server/middleware/security.js | 2 +- 10 files changed, 47 insertions(+), 45 deletions(-) diff --git a/client/index.js b/client/index.js index fff9f4f70650fa..193f8b75a01a74 100644 --- a/client/index.js +++ b/client/index.js @@ -54,14 +54,11 @@ renderApp(DemoApp); require('./registerServiceWorker'); // The following is needed so that we can support hot reloading our application. -if (process.env.BUILD_FLAG_IS_DEV && module.hot) { +if (process.env.BUILD_FLAG_IS_DEV === 'true' && module.hot) { // Accept changes to this file for hot reloading. module.hot.accept('./index.js'); // Any changes to our App will cause a hotload re-render. - module.hot.accept( - '../shared/components/DemoApp', - () => { - renderApp(require('../shared/components/DemoApp').default); - }, - ); + module.hot.accept('../shared/components/DemoApp', () => { + renderApp(require('../shared/components/DemoApp').default); + }); } diff --git a/client/registerServiceWorker.js b/client/registerServiceWorker.js index 636badb5004765..af402f6af429d8 100644 --- a/client/registerServiceWorker.js +++ b/client/registerServiceWorker.js @@ -10,10 +10,11 @@ import config from '../config'; -if (!process.env.BUILD_FLAG_IS_DEV) { +if (process.env.BUILD_FLAG_IS_DEV === 'false') { // We check the shared config, ensuring that the service worker has been // enabled. if (config('serviceWorker.enabled')) { + // eslint-disable-next-line global-require const OfflinePluginRuntime = require('offline-plugin/runtime'); // Install the offline plugin, which instantiates our service worker and app diff --git a/config/index.js b/config/index.js index 8fb17c6c4088d6..a847ddd9743ba0 100644 --- a/config/index.js +++ b/config/index.js @@ -34,7 +34,10 @@ function resolveConfigForBrowserOrServer() { // will be removed when "process.env.BUILD_FLAG_IS_NODE === true". // If no "BUILD_FLAG_IS_NODE" env var is undefined we can assume that we are running outside // of a webpack run, and will therefore return the config file. - if (typeof process.env.BUILD_FLAG_IS_NODE === 'undefined' || process.env.BUILD_FLAG_IS_NODE) { + if ( + typeof process.env.BUILD_FLAG_IS_NODE === 'undefined' || + process.env.BUILD_FLAG_IS_NODE === 'true' + ) { // i.e. running in our server/node process. configCache = require('./values').default; return configCache; @@ -42,8 +45,7 @@ function resolveConfigForBrowserOrServer() { // To get here we are likely running in the browser. - if (typeof window !== 'undefined' - && typeof window.__CLIENT_CONFIG__ === 'object') { + if (typeof window !== 'undefined' && typeof window.__CLIENT_CONFIG__ === 'object') { configCache = window.__CLIENT_CONFIG__; } else { // To get here we must be running in the browser. @@ -87,20 +89,22 @@ function resolveConfigForBrowserOrServer() { * could not be found at the given path. */ export default function configGet(path) { - const parts = typeof path === 'string' - ? path.split('.') - : path; + const parts = typeof path === 'string' ? path.split('.') : path; if (parts.length === 0) { - throw new Error('You must provide the path to the configuration value you would like to consume.'); + throw new Error( + 'You must provide the path to the configuration value you would like to consume.', + ); } let result = resolveConfigForBrowserOrServer(); for (let i = 0; i < parts.length; i += 1) { if (result === undefined) { const errorMessage = `Failed to resolve configuration value at "${parts.join('.')}".`; // This "if" block gets stripped away by webpack for production builds. - if (process.env.BUILD_FLAG_IS_DEV && process.env.BUILD_FLAG_IS_CLIENT) { - throw new Error(`${errorMessage} We have noticed that you are trying to access this configuration value from the client bundle (i.e. code that will be executed in a browser). For configuration values to be exposed to the client bundle you must ensure that the path is added to the client configuration filter in the project configuration values file.`); + if (process.env.BUILD_FLAG_IS_DEV === 'true' && process.env.BUILD_FLAG_IS_CLIENT === 'true') { + throw new Error( + `${errorMessage} We have noticed that you are trying to access this configuration value from the client bundle (i.e. code that will be executed in a browser). For configuration values to be exposed to the client bundle you must ensure that the path is added to the client configuration filter in the project configuration values file.`, + ); } throw new Error(errorMessage); } diff --git a/config/values.js b/config/values.js index 893e215a7e7bcc..8f35ea2546a015 100644 --- a/config/values.js +++ b/config/values.js @@ -83,10 +83,7 @@ const values = { childSrc: [], connectSrc: [], defaultSrc: [], - fontSrc: [ - 'https://fonts.googleapis.com/css', - 'https://fonts.gstatic.com', - ], + fontSrc: ['https://fonts.googleapis.com/css', 'https://fonts.gstatic.com'], imgSrc: [], mediaSrc: [], manifestSrc: [], @@ -232,11 +229,7 @@ const values = { srcEntryFile: './server/index.js', // Src paths. - srcPaths: [ - './server', - './shared', - './config', - ], + srcPaths: ['./server', './shared', './config'], // Where does the server bundle output live? outputPath: './build/server', @@ -330,8 +323,10 @@ const values = { // This protects us from accidentally including this configuration in our // client bundle. That would be a big NO NO to do. :) -if (process.env.BUILD_FLAG_IS_CLIENT) { - throw new Error("You shouldn't be importing the `/config/values.js` directly into code that will be included in your 'client' bundle as the configuration object will be sent to user's browsers. This could be a security risk! Instead, use the `config` helper function located at `/config/index.js`."); +if (process.env.BUILD_FLAG_IS_CLIENT === 'true') { + throw new Error( + "You shouldn't be importing the `/config/values.js` directly into code that will be included in your 'client' bundle as the configuration object will be sent to user's browsers. This could be a security risk! Instead, use the `config` helper function located at `/config/index.js`.", + ); } export default values; diff --git a/internal/webpack/configFactory.js b/internal/webpack/configFactory.js index 692a239e98a157..28830a1051f1b6 100644 --- a/internal/webpack/configFactory.js +++ b/internal/webpack/configFactory.js @@ -227,14 +227,14 @@ export default function webpackConfigFactory(buildOptions) { // the associated values below. // // For example you may have the following in your code: - // if (process.env.BUILD_FLAG_IS_CLIENT === true) { + // if (process.env.BUILD_FLAG_IS_CLIENT === 'true') { // console.log('Foo'); // } // // If the BUILD_FLAG_IS_CLIENT was assigned a value of `false` the above // code would be converted to the following by the webpack bundling // process: - // if (false === true) { + // if ('false' === 'true') { // console.log('Foo'); // } // @@ -243,18 +243,22 @@ export default function webpackConfigFactory(buildOptions) { // final output. This is helpful for extreme cases where you want to // ensure that code is only included/executed on specific targets, or for // doing debugging. + // + // NOTE: We are stringifying the values to keep them in line with the + // expected type of a typical process.env member (i.e. string). + // @see https://github.com/ctrlplusb/react-universally/issues/395 new webpack.EnvironmentPlugin({ // It is really important to use NODE_ENV=production in order to use // optimised versions of some node_modules, such as React. NODE_ENV: isProd ? 'production' : 'development', // Is this the "client" bundle? - BUILD_FLAG_IS_CLIENT: isClient, + BUILD_FLAG_IS_CLIENT: JSON.stringify(isClient), // Is this the "server" bundle? - BUILD_FLAG_IS_SERVER: isServer, + BUILD_FLAG_IS_SERVER: JSON.stringify(isServer), // Is this a node bundle? - BUILD_FLAG_IS_NODE: isNode, + BUILD_FLAG_IS_NODE: JSON.stringify(isNode), // Is this a development build? - BUILD_FLAG_IS_DEV: isDev, + BUILD_FLAG_IS_DEV: JSON.stringify(isDev), }), // Generates a JSON file containing a map of all the output files for diff --git a/server/index.js b/server/index.js index 4704a56f567a6c..d02377d32aa6d3 100644 --- a/server/index.js +++ b/server/index.js @@ -27,7 +27,7 @@ app.use(compression()); // Register our service worker generated by our webpack config. // We do not want the service worker registered for development builds, and // additionally only want it registered if the config allows. -if (!process.env.BUILD_FLAG_IS_DEV && config('serviceWorker.enabled')) { +if (process.env.BUILD_FLAG_IS_DEV === 'false' && config('serviceWorker.enabled')) { app.get(`/${config('serviceWorker.fileName')}`, serviceWorker); app.get( `${config('bundles.client.webPath')}${config('serviceWorker.offlinePageFileName')}`, diff --git a/server/middleware/reactApplication/ServerHTML.js b/server/middleware/reactApplication/ServerHTML.js index bb075afdb9932c..c0caf485295035 100644 --- a/server/middleware/reactApplication/ServerHTML.js +++ b/server/middleware/reactApplication/ServerHTML.js @@ -83,7 +83,9 @@ function ServerHTML(props) { // generate a vendor DLL in order to dramatically reduce our // compilation times. Therefore we need to inject the path to the // vendor dll bundle below. - ifElse(process.env.BUILD_FLAG_IS_DEV && config('bundles.client.devVendorDLL.enabled'))(() => + ifElse( + process.env.BUILD_FLAG_IS_DEV === 'true' && config('bundles.client.devVendorDLL.enabled'), + )(() => scriptTag( `${config('bundles.client.webPath')}${config('bundles.client.devVendorDLL.name')}.js?t=${Date.now()}`, )), diff --git a/server/middleware/reactApplication/getClientBundleEntryAssets.js b/server/middleware/reactApplication/getClientBundleEntryAssets.js index d6a5e9c996c7ec..83d4ab8629bab5 100644 --- a/server/middleware/reactApplication/getClientBundleEntryAssets.js +++ b/server/middleware/reactApplication/getClientBundleEntryAssets.js @@ -26,7 +26,7 @@ export default function getClientBundleEntryAssets() { // Return the assets json cache if it exists. // In development mode we always read the assets json file from disk to avoid // any cases where an older version gets cached. - if (!process.env.BUILD_FLAG_IS_DEV && resultCache) { + if (process.env.BUILD_FLAG_IS_DEV === 'false' && resultCache) { return resultCache; } diff --git a/server/middleware/reactApplication/index.js b/server/middleware/reactApplication/index.js index f2ab2af8f20973..62be80e15236d2 100644 --- a/server/middleware/reactApplication/index.js +++ b/server/middleware/reactApplication/index.js @@ -1,4 +1,3 @@ - import React from 'react'; import Helmet from 'react-helmet'; import { renderToString, renderToStaticMarkup } from 'react-dom/server'; @@ -25,7 +24,7 @@ export default function reactApplicationMiddleware(request, response) { // It's possible to disable SSR, which can be useful in development mode. // In this case traditional client side only rendering will occur. if (config('disableSSR')) { - if (process.env.BUILD_FLAG_IS_DEV) { + if (process.env.BUILD_FLAG_IS_DEV === 'true') { // eslint-disable-next-line no-console console.log('==> Handling react route without SSR'); } @@ -78,11 +77,11 @@ export default function reactApplicationMiddleware(request, response) { response .status( reactRouterContext.missed - // If the renderResult contains a "missed" match then we set a 404 code. - // Our App component will handle the rendering of an Error404 view. - ? 404 - // Otherwise everything is all good and we send a 200 OK status. - : 200, + ? // If the renderResult contains a "missed" match then we set a 404 code. + // Our App component will handle the rendering of an Error404 view. + 404 + : // Otherwise everything is all good and we send a 200 OK status. + 200, ) .send(`${html}`); }); diff --git a/server/middleware/security.js b/server/middleware/security.js index 6999d94de937bf..0b12fe642b9971 100644 --- a/server/middleware/security.js +++ b/server/middleware/security.js @@ -55,7 +55,7 @@ Object.keys(cspExtensions).forEach((key) => { } }); -if (process.env.BUILD_FLAG_IS_DEV) { +if (process.env.BUILD_FLAG_IS_DEV === 'true') { // When in development mode we need to add our secondary express server that // is used to host our client bundle to our csp config. Object.keys(cspConfig.directives).forEach((directive) => {