Repository: mapbox/batfish
Pull request #192 (2017-10-11 09:08:02)

Allow assets path to be configurable
`assets` folder can now be customized by setting `publicAssetsPath` in batfish.config.

This closes https://github.com/mapbox/batfish/issues/191

davidtheclark: Well done, @sudhakar! You got the feature itself, option validation, documentation, and a bunch of tests. So good.

I have just one more request to finish it off: let's make sure there are at least one or two tests that will break if this option doesn't work as expected. In the tests for `build` there should be some space to try a custom `publicAssetsPath` and assert that it has the intended effect.
sudhakar: Thanks for kind words @davidtheclark ;)

Added few tests to `clientWebpack` & `staticWebpack` config. Let me know if you want to cover any other scenarios.
davidtheclark: 👍 I'll get a release out soon with this feature.

Patch:
diff --git a/docs/configuration.md b/docs/configuration.md
index 79bf7ac..8509ce4 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -23,6 +23,7 @@ You can specify an alternate location.
 - [Basic options](#basic-options)
   - [siteBasePath](#sitebasepath)
   - [siteOrigin](#siteorigin)
+  - [publicAssetsPath](#publicassetspath)
   - [applicationWrapperPath](#applicationwrapperpath)
   - [stylesheets](#stylesheets)
   - [browserslist](#browserslist)
@@ -79,6 +80,13 @@ Origin where the site will be deployed.
 
 Also, *required if you want a sitemap*.
 
+### publicAssetsPath
+
+Type: `string`.
+Default: `'assets'`
+
+Default folder where batfish assets will be placed for static webpack build (aka `npm run build`)
+
 ### applicationWrapperPath
 
 Type: `string`.
diff --git a/flow-typed/defs.js b/flow-typed/defs.js
index 6b2b528..9135ebd 100644
--- a/flow-typed/defs.js
+++ b/flow-typed/defs.js
@@ -12,6 +12,7 @@ declare type JsonValue =
 declare type BatfishConfiguration = {
   siteBasePath: string,
   siteOrigin?: string,
+  publicAssetsPath: string,
   applicationWrapperPath: string,
   stylesheets: Array<string | Array<string>>,
   browserslist: string | Array<string>,
diff --git a/src/node/build-html.js b/src/node/build-html.js
index 0291ebf..2345329 100644
--- a/src/node/build-html.js
+++ b/src/node/build-html.js
@@ -6,7 +6,6 @@ const path = require('path');
 const fs = require('fs');
 const pTry = require('p-try');
 const joinUrlParts = require('./join-url-parts');
-const constants = require('./constants');
 const errorTypes = require('./error-types');
 const wrapError = require('./wrap-error');
 const UglifyJs = require('uglify-js');
@@ -31,7 +30,7 @@ function buildHtml(
   return pTry(() => {
     const assetsDirectory = path.join(
       batfishConfig.outputDirectory,
-      constants.PUBLIC_PATH_ASSETS
+      batfishConfig.publicAssetsPath
     );
 
     // This file reading is synced to make scoping easier, and with so few
@@ -58,7 +57,7 @@ function buildHtml(
     if (!_.isEmpty(batfishConfig.stylesheets) && cssFilename) {
       cssUrl = joinUrlParts(
         batfishConfig.siteBasePath,
-        constants.PUBLIC_PATH_ASSETS,
+        batfishConfig.publicAssetsPath,
         path.basename(cssFilename)
       );
     }
diff --git a/src/node/build.js b/src/node/build.js
index 8512ee8..564e003 100644
--- a/src/node/build.js
+++ b/src/node/build.js
@@ -43,7 +43,7 @@ function build(rawConfig?: Object, projectDirectory?: string): EventEmitter {
   const outputDirectory = batfishConfig.outputDirectory;
   const assetsDirectory = path.join(
     outputDirectory,
-    constants.PUBLIC_PATH_ASSETS
+    batfishConfig.publicAssetsPath
   );
 
   // For the static build, put everything Webpack makes in an assets/ subdirectory.
diff --git a/src/node/constants.js b/src/node/constants.js
index ed82da2..1798bdf 100644
--- a/src/node/constants.js
+++ b/src/node/constants.js
@@ -5,7 +5,6 @@ module.exports = Object.freeze({
   INLINE_CSS_MARKER: '<!-- INLINE CSS HERE, BATFISH -->',
   BATFISH_CSS_BASENAME: 'batfish-styles.css',
   STATS_BASENAME: 'stats.json',
-  PUBLIC_PATH_ASSETS: 'assets',
   DATA_DIRECTORY: 'data',
   EVENT_ERROR: 'error',
   EVENT_NOTIFICATION: 'notification',
diff --git a/src/node/create-webpack-config-base.js b/src/node/create-webpack-config-base.js
index f4b2b56..a121ffe 100644
--- a/src/node/create-webpack-config-base.js
+++ b/src/node/create-webpack-config-base.js
@@ -155,7 +155,7 @@ function createWebpackConfigBase(
         path: batfishConfig.outputDirectory,
         publicPath: joinUrlParts(
           batfishConfig.siteBasePath,
-          constants.PUBLIC_PATH_ASSETS,
+          batfishConfig.publicAssetsPath,
           ''
         ),
         pathinfo: !batfishConfig.production,
diff --git a/src/node/get-postcss-plugins.js b/src/node/get-postcss-plugins.js
index 395c8a4..e786ef2 100644
--- a/src/node/get-postcss-plugins.js
+++ b/src/node/get-postcss-plugins.js
@@ -5,7 +5,6 @@ const postcssUrl = require('postcss-url');
 const postcssCsso = require('postcss-csso');
 const url = require('url');
 const joinUrlParts = require('./join-url-parts');
-const constants = require('./constants');
 
 function getPostcssPlugins(
   batfishConfig: BatfishConfiguration
@@ -27,7 +26,7 @@ function getPostcssPlugins(
         }
         return joinUrlParts(
           batfishConfig.siteBasePath,
-          constants.PUBLIC_PATH_ASSETS,
+          batfishConfig.publicAssetsPath,
           asset.url
         );
       }
diff --git a/src/node/start.js b/src/node/start.js
index 5f62e87..752b18b 100644
--- a/src/node/start.js
+++ b/src/node/start.js
@@ -36,7 +36,7 @@ function start(rawConfig?: Object, projectDirectory?: string): EventEmitter {
 
   const serveAssetsDir = joinUrlParts(
     batfishConfig.siteBasePath,
-    constants.PUBLIC_PATH_ASSETS
+    batfishConfig.publicAssetsPath
   );
   // This allows us to serve static files within the pages directory.
   const servePagesDir = batfishConfig.siteBasePath;
diff --git a/src/node/validate-config.js b/src/node/validate-config.js
index 6ab6161..4eca209 100644
--- a/src/node/validate-config.js
+++ b/src/node/validate-config.js
@@ -25,6 +25,10 @@ const configSchema = {
     validator: _.isString,
     description: 'string'
   },
+  publicAssetsPath: {
+    validator: _.isString,
+    description: 'string'
+  },
   applicationWrapperPath: {
     validator: isAbsolutePath,
     description: 'absolute path'
@@ -180,6 +184,7 @@ function validateConfig(
     production: false,
     verbose: false,
     port: 8080,
+    publicAssetsPath: 'assets',
     pagesDirectory: path.join(projectDirectory, 'src/pages'),
     outputDirectory: path.join(projectDirectory, '_batfish_site'),
     temporaryDirectory: path.join(projectDirectory, '_batfish_tmp'),


Test patch:
diff --git a/test/__snapshots__/validate-config.test.js.snap b/test/__snapshots__/validate-config.test.js.snap
index 517dea3..c9086b7 100644
--- a/test/__snapshots__/validate-config.test.js.snap
+++ b/test/__snapshots__/validate-config.test.js.snap
@@ -33,6 +33,7 @@ Object {
   ],
   "production": false,
   "productionDevtool": false,
+  "publicAssetsPath": "assets",
   "siteBasePath": "",
   "stylesheets": Array [],
   "temporaryDirectory": "/my-project/_batfish_tmp",
diff --git a/test/build-html.test.js b/test/build-html.test.js
index 729f229..24b8d6d 100644
--- a/test/build-html.test.js
+++ b/test/build-html.test.js
@@ -39,6 +39,7 @@ describe('buildHtml', () => {
   beforeEach(() => {
     batfishConfig = {
       outputDirectory: '/mock/output',
+      publicAssetsPath: 'assets',
       siteBasePath: '/base',
       stylesheets: ['one.css', 'two.css']
     };
diff --git a/test/build.test.js b/test/build.test.js
index c264a08..c8d2483 100644
--- a/test/build.test.js
+++ b/test/build.test.js
@@ -70,6 +70,7 @@ describe('build', () => {
   beforeEach(() => {
     validateConfig.mockValidatedConfig = {
       outputDirectory: '/mock/output',
+      publicAssetsPath: 'assets',
       production: true,
       siteOrigin: 'https://www.mapbox.com',
       verbose: false
@@ -107,6 +108,40 @@ describe('build', () => {
     );
   });
 
+  test('creates client webpack config with custom publicAssetsPath', done => {
+    validateConfig.mockValidatedConfig = {
+      outputDirectory: '/mock/output',
+      publicAssetsPath: 'site_assets'
+    };
+    const emitter = build();
+    emitter.on(constants.EVENT_ERROR, logEmitterError);
+    process.nextTick(() => {
+      expect(createWebpackConfigClient).toHaveBeenCalledTimes(1);
+      expect(createWebpackConfigClient).toHaveBeenCalledWith({
+        outputDirectory: '/mock/output/site_assets',
+        publicAssetsPath: 'site_assets'
+      });
+      done();
+    });
+  });
+
+  test('creates static webpack config with custom publicAssetsPath', done => {
+    validateConfig.mockValidatedConfig = {
+      outputDirectory: '/mock/output',
+      publicAssetsPath: 'site_assets'
+    };
+    const emitter = build();
+    emitter.on(constants.EVENT_ERROR, logEmitterError);
+    process.nextTick(() => {
+      expect(createWebpackConfigStatic).toHaveBeenCalledTimes(1);
+      expect(createWebpackConfigStatic).toHaveBeenCalledWith({
+        outputDirectory: '/mock/output/site_assets',
+        publicAssetsPath: 'site_assets'
+      });
+      done();
+    });
+  });
+
   test('catches errors while validating config', done => {
     const expectedError = new Error();
     validateConfig.mockImplementationOnce(() => {
@@ -169,6 +204,7 @@ describe('build', () => {
       expect(createWebpackConfigClient).toHaveBeenCalledTimes(1);
       expect(createWebpackConfigClient).toHaveBeenCalledWith({
         outputDirectory: '/mock/output/assets',
+        publicAssetsPath: 'assets',
         production: true,
         siteOrigin: 'https://www.mapbox.com',
         verbose: false
@@ -240,6 +276,7 @@ describe('build', () => {
       expect(createWebpackConfigStatic).toHaveBeenCalledTimes(1);
       expect(createWebpackConfigStatic).toHaveBeenCalledWith({
         outputDirectory: '/mock/output/assets',
+        publicAssetsPath: 'assets',
         production: true,
         siteOrigin: 'https://www.mapbox.com',
         verbose: false
diff --git a/test/compile-stylesheets.test.js b/test/compile-stylesheets.test.js
index a003b76..b84574d 100644
--- a/test/compile-stylesheets.test.js
+++ b/test/compile-stylesheets.test.js
@@ -35,6 +35,7 @@ describe('compileStylesheets', () => {
 
     batfishConfig = {
       outputDirectory: tmp,
+      publicAssetsPath: 'assets',
       // Test URLs, globs, and absolute filenames.
       stylesheets: [
         'https://www.mapbox.com/mock-style.css',
diff --git a/test/start.test.js b/test/start.test.js
index ce4c383..556f9e0 100644
--- a/test/start.test.js
+++ b/test/start.test.js
@@ -55,6 +55,7 @@ describe('start', () => {
   beforeEach(() => {
     validateConfig.mockValidatedConfig = {
       port: 6666,
+      publicAssetsPath: 'assets',
       outputDirectory: '/mock/output',
       pagesDirectory: '/mock/pages'
     };
diff --git a/test/validate-config.test.js b/test/validate-config.test.js
index a7ae56e..fb1b11a 100644
--- a/test/validate-config.test.js
+++ b/test/validate-config.test.js
@@ -121,6 +121,22 @@ describe('validateConfig', () => {
     ).toHaveProperty('siteOrigin', 'https://www.mapbox.com');
   });
 
+  test('publicAssetsPath should be configurable', () => {
+    expect(validateConfig(undefined, projectDirectory)).toHaveProperty(
+      'publicAssetsPath',
+      'assets'
+    );
+
+    expect(
+      validateConfig(
+        {
+          publicAssetsPath: 'site_assets'
+        },
+        projectDirectory
+      )
+    ).toHaveProperty('publicAssetsPath', 'site_assets');
+  });
+
   test('processed siteBasePath does not end with a slash unless it is only a slash', () => {
     expect(
       validateConfig(