Skip to content

Commit

Permalink
Changes process.env build flags to be inline with expected string typ…
Browse files Browse the repository at this point in the history
…e of typical process.env member. closes mui#395
  • Loading branch information
ctrlplusb committed Apr 5, 2017
1 parent 7c5338b commit e977ce2
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 45 deletions.
11 changes: 4 additions & 7 deletions client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
3 changes: 2 additions & 1 deletion client/registerServiceWorker.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 13 additions & 9 deletions config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,18 @@ 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;
}

// 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.
Expand Down Expand Up @@ -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);
}
Expand Down
17 changes: 6 additions & 11 deletions config/values.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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 `<projectroot>/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 `<projectroot>/config/index.js`.");
if (process.env.BUILD_FLAG_IS_CLIENT === 'true') {
throw new Error(
"You shouldn't be importing the `<projectroot>/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 `<projectroot>/config/index.js`.",
);
}

export default values;
16 changes: 10 additions & 6 deletions internal/webpack/configFactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
// }
//
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')}`,
Expand Down
4 changes: 3 additions & 1 deletion server/middleware/reactApplication/ServerHTML.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()}`,
)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
13 changes: 6 additions & 7 deletions server/middleware/reactApplication/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

import React from 'react';
import Helmet from 'react-helmet';
import { renderToString, renderToStaticMarkup } from 'react-dom/server';
Expand All @@ -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');
}
Expand Down Expand Up @@ -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(`<!DOCTYPE html>${html}`);
});
Expand Down
2 changes: 1 addition & 1 deletion server/middleware/security.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down

0 comments on commit e977ce2

Please sign in to comment.