ssr client config instead of the server config. that way secrets dont…

… get sent to client also makes it so bundler isn't run on make run.
Automattic · Feb 12, 2017 · e9c32ff · e9c32ff
1 parent a1a8134
commit e9c32ff
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 38 deletions.
diff --git a/Makefile b/Makefile
@@ -164,7 +164,7 @@ build-server: install
 	@CALYPSO_ENV=$(CALYPSO_ENV) $(NODE_BIN)/webpack --display-error-details --config webpack.config.node.js
 
 build: install build-server build-dll build-css server/devdocs/search-index.js server/devdocs/proptypes-index.json server/devdocs/components-usage-stats.json
-	@$(BUNDLER)
+	@if [ $(CALYPSO_ENV) != development ]; then $(BUNDLER); fi
 
 build-css: public/style.css public/style-rtl.css public/style-debug.css public/editor.css
 

diff --git a/server/config/index.js b/server/config/index.js
@@ -1,8 +1,9 @@
 const template = require( 'lodash/template' );
 const configPath = require( 'path' ).resolve( __dirname, '..', '..', 'config' );
-const data = require( './parser' )( configPath, {
+const parser = require( './parser' );
+
+const { serverData: data, clientData } = parser( configPath, {
 	env: process.env.CALYPSO_ENV || process.env.NODE_ENV || 'development',
-	includeSecrets: true,
 	enabledFeatures: process.env.ENABLE_FEATURES,
 	disabledFeatures: process.env.DISABLE_FEATURES
 } );
@@ -39,7 +40,7 @@ const ssrConfig = template(
 	'<%= isEnabled %>' +
 	'<%= anyEnabled %>'
 ) ( {
-	data: JSON.stringify( data ),
+	data: JSON.stringify( clientData ),
 	config: config.toString(),
 	isEnabled: isEnabled.toString(),
 	anyEnabled: anyEnabled.toString(),
@@ -48,4 +49,4 @@ const ssrConfig = template(
 module.exports = config;
 module.exports.isEnabled = isEnabled;
 module.exports.anyEnabled = anyEnabled;
-module.exports.srrConfig = ssrConfig;
+module.exports.ssrConfig = ssrConfig;
diff --git a/server/config/parser.js b/server/config/parser.js
@@ -24,28 +24,19 @@ function getDataFromFile( file ) {
 module.exports = function( configPath, defaultOpts ) {
 	var opts = assign( {
 			env: 'development',
-			includeSecrets: false,
 		}, defaultOpts ),
 		data = {},
 		configFiles = [
 			path.resolve( configPath, '_shared.json' ),
 			path.resolve( configPath, opts.env + '.json' ),
 			path.resolve( configPath, opts.env + '.local.json' )
 		],
-		realSecretsPath,
-		emptySecretsPath,
-		secretsPath,
+		realSecretsPath = path.resolve( configPath, 'secrets.json' ),
+		emptySecretsPath = path.resolve( configPath, 'empty-secrets.json' ),
+		secretsPath = fs.existsSync( realSecretsPath ) ? realSecretsPath : emptySecretsPath,
 		enabledFeatures = opts.enabledFeatures ? opts.enabledFeatures.split( ',' ) : [],
 		disabledFeatures = opts.disabledFeatures ? opts.disabledFeatures.split( ',' ) : [];
 
-	if ( opts.includeSecrets ) {
-		realSecretsPath = path.resolve( configPath, 'secrets.json' );
-		emptySecretsPath = path.resolve( configPath, 'empty-secrets.json' );
-		secretsPath = fs.existsSync( realSecretsPath ) ? realSecretsPath : emptySecretsPath;
-
-		configFiles.push( secretsPath );
-	}
-
 	configFiles.forEach( function( file ) {
 		assign( data, getDataFromFile( file ) );
 	} );
@@ -61,5 +52,8 @@ module.exports = function( configPath, defaultOpts ) {
 		} );
 	}
 
-	return data;
+	const serverData = assign( {}, data, getDataFromFile( secretsPath ) );
+	const clientData = assign( {}, data );
+
+	return { serverData, clientData };
 }
diff --git a/server/config/test/parser.js b/server/config/test/parser.js
@@ -27,40 +27,30 @@ describe( 'parser', () => {
 		mockery.resetCache(); // reset require cache
 	} );
 
-	it( 'should return empty object for invalid path', () => {
+	it( 'should return empty objects for an invalid path', () => {
 		mockery.registerMock( 'fs', mocks.INVALID_PATH );
 		parser = require( 'config/parser' );
 
-		let data = parser( '/invalid-path' );
+		const data = parser( '/invalid-path' );
 
-		expect( data ).to.eql( {} );
+		expect( data ).to.eql( { serverData: {}, clientData: {} } );
 	} );
 
-	it( 'should not include secrets by default', () => {
+	it( 'server should have secrets and client should not', () => {
 		mockery.registerMock( 'fs', mocks.VALID_SECRETS );
 		parser = require( 'config/parser' );
 
-		let data = parser( '/valid-path' );
+		const data = parser( '/valid-path' );
 
-		expect( data ).to.not.have.property( 'secret' );
-	} );
-
-	it( 'should include secrets when `includeSecrets` is true', () => {
-		mockery.registerMock( 'fs', mocks.VALID_SECRETS );
-		parser = require( 'config/parser' );
-
-		let data = parser( '/valid-path', {
-			includeSecrets: true
-		} );
-
-		expect( data ).to.have.property( 'secret', 'very' );
+		expect( data.clientData ).to.not.have.property( 'secret' );
+		expect( data.serverData ).to.have.property( 'secret' );
 	} );
 
 	it( 'should cascade configs', () => {
 		mockery.registerMock( 'fs', mocks.VALID_ENV_FILES );
 		parser = require( 'config/parser' );
 
-		let data = parser( '/valid-path', {
+		const { serverData: data } = parser( '/valid-path', {
 			env: 'myenv'
 		} );
 
@@ -83,7 +73,7 @@ describe( 'parser', () => {
 		mockery.registerMock( 'fs', mocks.VALID_ENV_FILES );
 		parser = require( 'config/parser' );
 
-		let data = parser( '/valid-path', {
+		const { serverData: data } = parser( '/valid-path', {
 			env: 'myenv',
 			disabledFeatures: 'enabledFeature2'
 		} );
@@ -95,7 +85,7 @@ describe( 'parser', () => {
 		mockery.registerMock( 'fs', mocks.VALID_ENV_FILES );
 		parser = require( 'config/parser' );
 
-		let data = parser( '/valid-path', {
+		const { serverData: data } = parser( '/valid-path', {
 			env: 'myenv',
 			enabledFeatures: 'disabledFeature2'
 		} );