Repository: mapbox/batfish
Pull request #280 (2018-05-23 18:33:52)

Fix custom 404 pages on sites with a siteBasePath
If you had a `siteBasePath` and created a custom 404 page, you'd get some weird build errors: the `_404.js` file in the temporary directory was getting all scrambled. The root cause was that the 404 logic in `getPagesData` didn't take `siteBasePath` into account. This fixes that.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1eb7884..91ab3c9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## HEAD
+
+- **Fix:** Fix custom 404 pages on sites with a `siteBasePath`.
+
 ## 1.9.0
 
 - **Feature:** Added a helpful message if the provided port is not available.
diff --git a/src/node/get-pages-data.js b/src/node/get-pages-data.js
index 44db856..c042d31 100644
--- a/src/node/get-pages-data.js
+++ b/src/node/get-pages-data.js
@@ -21,6 +21,9 @@ const wrapError = require('./wrap-error');
 function getPagesData(
   batfishConfig: BatfishConfiguration
 ): Promise<{ [string]: BatfishPageData }> {
+  const base = batfishConfig.siteBasePath;
+  const notFoundPath = joinUrlParts(base, '404', '');
+
   // Convert a page's file path to its URL path.
   const pageFilePathToUrlPath = (filePath: string): string => {
     const relativePath = path.relative(batfishConfig.pagesDirectory, filePath);
@@ -64,13 +67,12 @@ function getPagesData(
       };
       if (is404) {
         pageData.is404 = true;
-        pageData.path = '/404/';
+        pageData.path = notFoundPath;
       }
       pagesData[pagePath] = pageData;
     });
   };
 
-  const base = batfishConfig.siteBasePath;
   const pagesGlob = [
     path.join(batfishConfig.pagesDirectory, `**/*.${constants.PAGE_EXT_GLOB}`)
   ];
@@ -98,13 +100,13 @@ function getPagesData(
     })
     .then(() => {
       if (
-        !pagesData['/404/'] &&
+        !pagesData[notFoundPath] &&
         !batfishConfig.production &&
         !batfishConfig.spa
       ) {
-        pagesData['/404/'] = {
+        pagesData[notFoundPath] = {
           filePath: path.join(__dirname, '../webpack/default-not-found.js'),
-          path: '/404/',
+          path: notFoundPath,
           is404: true,
           frontMatter: {}
         };


Test patch:
diff --git a/test/get-pages-data.test.js b/test/get-pages-data.test.js
index 5ce7ee7..db569e6 100644
--- a/test/get-pages-data.test.js
+++ b/test/get-pages-data.test.js
@@ -189,6 +189,18 @@ describe('getPagesData', () => {
     });
   });
 
+  test('with siteBasePath, includes the default 404 page in development mode', () => {
+    const config = validateConfig({
+      pagesDirectory: fixtureDir,
+      siteBasePath: 'foo'
+    });
+    return getPagesData(config).then(result => {
+      expect(result['/foo/404/']).not.toBeUndefined();
+      expect(result['/foo/404/'].is404).toBe(true);
+      expect(result['/foo/404/'].filePath).toMatch(/default-not-found\.js$/);
+    });
+  });
+
   test('does not include the default 404 page in production mode', () => {
     const config = validateConfig({
       pagesDirectory: fixtureDir,
@@ -227,6 +239,26 @@ describe('getPagesData', () => {
     });
   });
 
+  test('with siteBasePath, does not include the default 404 page if there is a custom 404 JS page', () => {
+    const config = validateConfig({
+      pagesDirectory: path.join(__dirname, 'fixtures/get-pages-data-404-js'),
+      siteBasePath: 'foo/bar'
+    });
+    return getPagesData(config).then(result => {
+      expect(result['/foo/bar/404/']).not.toBeUndefined();
+      expect(result['/foo/bar/404/'].is404).toBe(true);
+      expect(result['/foo/bar/404/'].filePath).not.toMatch(
+        /default-not-found\.js$/
+      );
+      expect(result['/foo/bar/404/'].filePath).toMatch(
+        /get-pages-data-404-js\/404\.js$/
+      );
+      expect(result['/foo/bar/404/'].frontMatter).toEqual({
+        title: 'Not found'
+      });
+    });
+  });
+
   test('does not include the default 404 page if there is a custom 404 Markdown page', () => {
     const config = validateConfig({
       pagesDirectory: path.join(__dirname, 'fixtures/get-pages-data-404-md')