Simplify index file logic

The directoryIndexFile/indexPath logic was unnecessarily complicated, and would do the wrong thing if only indexPath was specified. This simlifies and fixes that issue.

Author: Ben Demboski

index.js

@@ -1,3 +1,4 @@
+const { join } = require('path');
 const createHandler = require('./lib/handler');
 
 function requiredParam(param, errorMessage) {
@@ -16,8 +17,7 @@ function requiredParam(param, errorMessage) {
  * @param  {Object} options.protocol           electron.protocol
  * @param  {String} options.name               name of your protocol, defaults to `serve`
  * @param  {String} options.endpoint           endpoint of your protocol, defaults to `dist`
- * @param  {String} options.directoryIndexFile directory index. usally the default, `index.html`
- * @param  {String} options.indexPath          defaults to cwd + directoryIndexFile
+ * @param  {String} options.indexPath          defaults to 'index.html' in cwd
  *
  * @return {String}                            name of your protocol
  */
@@ -27,15 +27,16 @@ module.exports = function protocolServe({
   protocol,
   name = 'serve',
   endpoint = 'dist',
-  directoryIndexFile = 'index.html',
   indexPath = undefined,
 }) {
   requiredParam(cwd, 'cwd must be specified, should be a valid path');
   requiredParam(protocol, 'protocol must be specified, should be electron.protocol');
   requiredParam(app, 'app must be specified, should be electron.app');
 
+  indexPath = indexPath || join(cwd, 'index.html');
+
   app.on('ready', () => {
-    const options = { cwd, name, endpoint, directoryIndexFile, indexPath };
+    const options = { cwd, name, endpoint, indexPath };
     protocol.registerFileProtocol(name, createHandler(options), error => {
       if (error) {
         console.error('Failed to register protocol');

lib/handler.js

@@ -6,10 +6,8 @@ module.exports = function createHandler({
   cwd,
   name,
   endpoint,
-  directoryIndexFile,
   indexPath,
 }) {
-  indexPath = indexPath || join(cwd, directoryIndexFile);
   const cache = {};
 
   return ({ url }, callback) => {
@@ -27,25 +25,22 @@ module.exports = function createHandler({
     }
 
     const pathSegments = pathname.split('/').filter(segment => segment !== '');
-
-    if (pathSegments.length === 0) {
-      pathSegments.push(directoryIndexFile);
-    }
-
     const filepath = join(cwd, ...pathSegments);
 
     // Basic request caching
     if (!cache[url]) {
       try {
         fs.accessSync(filepath);
 
-        cache[url] = filepath;
+        if (fs.statSync(filepath).isFile()) {
+          cache[url] = filepath;
+        }
       } catch (err) {
         //
       }
     }
 
-    // redirect unmet requests to directoryIndexFile
+    // redirect unmet requests to indexPath
     callback({ path: cache[url] || indexPath });
   };
 };

tests/unit/handler-test.js

@@ -8,15 +8,21 @@ const subject = require('../../lib/handler');
 describe('handler', () => {
   let sandbox;
   let mockFiles;
+  let mockDirs;
 
   before(() => {
     sandbox = sinon.sandbox.create();
     // Stub accessSync to act like only files in mockFiles exist
     sandbox.stub(fs, 'accessSync', path => {
-      if (mockFiles.indexOf(path) === -1) {
+      if (mockFiles.indexOf(path) === -1 && mockDirs.indexOf(path) === -1) {
         throw new Error('Doesn\'t exist');
       }
     });
+    sandbox.stub(fs, 'statSync', path => ({
+      isFile() {
+        return mockFiles.indexOf(path) !== -1;
+      },
+    }));
   });
 
   after(() => {
@@ -25,6 +31,7 @@ describe('handler', () => {
 
   beforeEach(() => {
     mockFiles = [];
+    mockDirs = [];
   });
 
   // Create a handler with the specified options
@@ -33,8 +40,7 @@ describe('handler', () => {
       cwd: '.',
       name: 'serve',
       endpoint: 'dist',
-      directoryIndexFile: 'index.html',
-      indexPath: undefined,
+      indexPath: join('.', 'index.html'),
     }, options));
   }
 
@@ -57,13 +63,15 @@ describe('handler', () => {
   }
 
   it('works', () => {
+    mockDirs.push('.');
     mockFiles.push('script.js');
     return handlerExec('serve://dist/script.js').then(path => {
       assert.equal(path, 'script.js');
     });
   });
 
   it('works with multiple requests', () => {
+    mockDirs.push('.');
     mockFiles.push('script1.js');
     mockFiles.push('script2.js');
 
@@ -77,25 +85,19 @@ describe('handler', () => {
     });
   });
 
-  it('serves directoryIndexFile from the root', () => {
+  it('serves indexPath from the root', () => {
+    mockDirs.push('.');
     mockFiles.push('foo.html');
     return handlerExec('serve://dist', {
-      directoryIndexFile: 'foo.html',
-    }).then(path => {
-      assert.equal(path, 'foo.html');
-    });
-  });
-
-  it('serves directoryIndexFile for missing files', () => {
-    mockFiles.push('foo.html');
-    return handlerExec('serve://dist/missing.js', {
-      directoryIndexFile: 'foo.html',
+      cwd: '.',
+      indexPath: join('.', 'foo.html'),
     }).then(path => {
-      assert.equal(path, 'foo.html');
+      assert.equal(path, join('.', 'foo.html'));
     });
   });
 
   it('respects relative cwd', () => {
+    mockDirs.push(join('foo', 'bar'));
     mockFiles.push(join('foo', 'bar', 'script.js'));
     return handlerExec('serve://dist/script.js', {
       cwd: join('foo', 'bar'),
@@ -105,6 +107,7 @@ describe('handler', () => {
   });
 
   it('respects absolute cwd', () => {
+    mockDirs.push(resolve('foo', 'bar'));
     mockFiles.push(resolve('foo', 'bar', 'script.js'));
     return handlerExec('serve://dist/script.js', {
       cwd: resolve('foo', 'bar'),
@@ -114,6 +117,7 @@ describe('handler', () => {
   });
 
   it('respects endpoint', () => {
+    mockDirs.push('.');
     mockFiles.push('script.js');
     return handlerExec('serve://custom/script.js', {
       endpoint: 'custom',
@@ -123,22 +127,26 @@ describe('handler', () => {
   });
 
   it('respects indexPath for missing files', () => {
+    mockDirs.push('bar');
     mockFiles.push(join('foo', 'bar.html'));
     return handlerExec('serve://dist/missing.js', {
+      cwd: 'bar',
       indexPath: join('foo', 'bar.html'),
     }).then(path => {
       assert.equal(path, join('foo', 'bar.html'));
     });
   });
 
   it('ignores hashes', () => {
+    mockDirs.push('.');
     mockFiles.push('script.js');
     return handlerExec('serve://dist/script.js#hash').then(path => {
       assert.equal(path, 'script.js');
     });
   });
 
   it('ignores query params', () => {
+    mockDirs.push('.');
     mockFiles.push('script.js');
     return handlerExec('serve://dist/script.js?query=param').then(path => {
       assert.equal(path, 'script.js');

tests/unit/index-test.js

@@ -2,7 +2,7 @@ const { assert } = require('chai');
 const mock = require('mock-require');
 const { join } = require('path');
 
-describe('handler', () => {
+describe('index', () => {
   let app;
   let protocol;
   let onAppReady;
@@ -51,8 +51,23 @@ describe('handler', () => {
       cwd: '.',
       name: 'serve',
       endpoint: 'dist',
-      directoryIndexFile: 'index.html',
-      indexPath: undefined,
+      indexPath: join('.', 'index.html'),
+    });
+  });
+
+  it('works with a custom cwd', () => {
+    register({
+      cwd: join('foo', 'bar'),
+    });
+    assert.ok(onAppReady);
+    onAppReady();
+
+    assert.equal(protocolName, 'serve');
+    assert.deepEqual(handlerOptions, {
+      cwd: join('foo', 'bar'),
+      name: 'serve',
+      endpoint: 'dist',
+      indexPath: join('foo', 'bar', 'index.html'),
     });
   });
 
@@ -61,8 +76,7 @@ describe('handler', () => {
       cwd: join('my', 'old'),
       name: 'friend',
       endpoint: 'so',
-      directoryIndexFile: 'we.html',
-      indexPath: join('meet', 'again'),
+      indexPath: join('we', 'meet', 'again.html'),
     });
     assert.ok(onAppReady);
     onAppReady();
@@ -72,8 +86,7 @@ describe('handler', () => {
       cwd: join('my', 'old'),
       name: 'friend',
       endpoint: 'so',
-      directoryIndexFile: 'we.html',
-      indexPath: join('meet', 'again'),
+      indexPath: join('we', 'meet', 'again.html'),
     });
   });