fix(server): Simplify 'dom' inclusion. (#3356)

johnjbarton · web-flow · commit 5f13e11784c2 · 2019-08-26T10:41:32.000-07:00
The karma preprocessor caches file content: no need to read it async in the server loop.
diff --git a/lib/middleware/karma.js b/lib/middleware/karma.js
@@ -66,7 +66,6 @@ function createKarmaMiddleware (
   filesPromise,
   serveStaticFile,
   serveFile,
-  readFilePromise,
   injector,
   basePath,
   urlRoot,
@@ -136,21 +135,8 @@ function createKarmaMiddleware (
     const isRequestingContextFile = requestUrl === '/context.html'
     const isRequestingDebugFile = requestUrl === '/debug.html'
     const isRequestingClientContextFile = requestUrl === '/client_with_context.html'
-    const includedContent = new Map() // file.path -> content
     if (isRequestingContextFile || isRequestingDebugFile || isRequestingClientContextFile) {
       return filesPromise.then((files) => {
-        // Read any files.included that will be directly written into HTML before HTML is read.
-        const contentReads = []
-        for (const file of files.included) {
-          const fileType = file.type || path.extname(file.path).substring(1)
-          if (fileType === 'dom') {
-            contentReads.push(
-              readFilePromise(file.path).then((content) => includedContent.set(file.path, content))
-            )
-          }
-        }
-        return Promise.all(contentReads).then(() => files)
-      }).then(function (files) {
         let fileServer
         let requestedFileUrl
         log.debug('custom files', customContextFile, customDebugFile, customClientContextFile)
@@ -195,7 +181,7 @@ function createKarmaMiddleware (
             if (fileType === 'css') {
               scriptTags.push(`<link type="text/css" href="${filePath}" rel="stylesheet">`)
             } else if (fileType === 'dom') {
-              scriptTags.push(includedContent.get(file.path))
+              scriptTags.push(file.content)
             } else if (fileType === 'html') {
               scriptTags.push(`<link href="${filePath}" rel="import">`)
             } else {
@@ -253,7 +239,6 @@ createKarmaMiddleware.$inject = [
   'filesPromise',
   'serveStaticFile',
   'serveFile',
-  'readFilePromise',
   'injector',
   'config.basePath',
   'config.urlRoot',
diff --git a/lib/web-server.js b/lib/web-server.js
@@ -39,25 +39,6 @@ function createFilesPromise (emitter, fileList) {
   return filesPromise
 }
 
-// Bind the filesystem into the injectable file reader function
-function createReadFilePromise () {
-  return (filepath) => {
-    return new Promise((resolve, reject) => {
-      fs.readFile(filepath, 'utf8', function (error, data) {
-        if (error) {
-          reject(new Error(`Cannot read ${filepath}, got: ${error}`))
-        } else if (!data) {
-          reject(new Error(`No content at ${filepath}`))
-        } else {
-          resolve(data.split('\n'))
-        }
-      })
-    })
-  }
-}
-
-createReadFilePromise.$inject = []
-
 function createServeStaticFile (config) {
   return common.createServeFile(fs, path.normalize(path.join(__dirname, '/../static')), config)
 }
@@ -124,6 +105,5 @@ module.exports = {
   createWebServer,
   createServeFile,
   createServeStaticFile,
-  createFilesPromise,
-  createReadFilePromise
+  createFilesPromise
 }
diff --git a/test/unit/middleware/karma.spec.js b/test/unit/middleware/karma.spec.js
@@ -12,15 +12,15 @@ const HttpRequestMock = mocks.http.ServerRequest
 
 describe('middleware.karma', () => {
   let serveFile
-  let readFilesDeferred
   let filesDeferred
   let nextSpy
   let response
 
   class MockFile extends File {
-    constructor (path, sha, type) {
+    constructor (path, sha, type, content) {
       super(path, undefined, undefined, type)
       this.sha = sha || 'sha-default'
+      this.content = content
     }
   }
 
@@ -64,7 +64,6 @@ describe('middleware.karma', () => {
       filesDeferred.promise,
       serveFile,
       null,
-      null,
       injector,
       '/base/path',
       '/__karma__/',
@@ -121,12 +120,10 @@ describe('middleware.karma', () => {
   })
 
   it('should serve client.html', (done) => {
-    readFilesDeferred = helper.defer()
     handler = createKarmaMiddleware(
       null,
       serveFile,
       null,
-      readFilesDeferred.promise,
       injector,
       '/base',
       '/'
@@ -142,12 +139,10 @@ describe('middleware.karma', () => {
   })
 
   it('should serve /?id=xxx', (done) => {
-    readFilesDeferred = helper.defer()
     handler = createKarmaMiddleware(
       null,
       serveFile,
       null,
-      readFilesDeferred.promise,
       injector,
       '/base',
       '/'
@@ -163,13 +158,10 @@ describe('middleware.karma', () => {
   })
 
   it('should serve /?x-ua-compatible with replaced values', (done) => {
-    readFilesDeferred = helper.defer()
-
     handler = createKarmaMiddleware(
       null,
       serveFile,
       null,
-      readFilesDeferred.promise,
       injector,
       '/base',
       '/'
@@ -278,31 +270,20 @@ describe('middleware.karma', () => {
   })
 
   it('should serve context.html with included DOM content', (done) => {
-    const readFilePromise = (path) => {
-      const cases = {
-        '/some/abc/a.dom': 'a',
-        '/some/abc/b_test_dom.html': 'b',
-        '/some/abc/c': 'c',
-        '/some/abc/d_test_dom.html': 'd'
-      }
-      return Promise.resolve(cases[path] || '?unknown ' + path)
-    }
-
     filesDeferred = helper.defer()
     handler = createKarmaMiddleware(
       filesDeferred.promise,
       serveFile,
       null,
-      readFilePromise,
       injector,
       '/base',
       '/'
     )
 
     includedFiles([
-      new MockFile('/some/abc/a.dom', 'sha1'),
-      new MockFile('/some/abc/b_test_dom.html', 'sha2', 'dom'),
-      new MockFile('/some/abc/c', 'sha3', 'dom')
+      new MockFile('/some/abc/a.dom', 'sha1', undefined, 'a'),
+      new MockFile('/some/abc/b_test_dom.html', 'sha2', 'dom', 'b'),
+      new MockFile('/some/abc/c', 'sha3', 'dom', 'c')
     ])
 
     response.once('end', () => {
@@ -468,12 +449,10 @@ describe('middleware.karma', () => {
 
   it('should update handle updated configs', (done) => {
     let i = 0
-    readFilesDeferred = helper.defer()
     handler = createKarmaMiddleware(
       filesDeferred.promise,
       serveFile,
       null,
-      readFilesDeferred.promise,
       {
         get (val) {
           if (val === 'config.client') {
diff --git a/test/unit/web-server.spec.js b/test/unit/web-server.spec.js
@@ -63,7 +63,6 @@ describe('web-server', () => {
         filesPromise: ['factory', m.createFilesPromise],
         serveStaticFile: ['factory', m.createServeStaticFile],
         serveFile: ['factory', m.createServeFile],
-        readFilePromise: ['factory', m.createReadFilePromise],
         capturedBrowsers: ['value', null],
         reporter: ['value', null],
         executor: ['value', null],
@@ -233,7 +232,6 @@ describe('web-server', () => {
         filesPromise: ['factory', m.createFilesPromise],
         serveStaticFile: ['factory', m.createServeStaticFile],
         serveFile: ['factory', m.createServeFile],
-        readFilePromise: ['factory', m.createReadFilePromise],
         capturedBrowsers: ['value', null],
         reporter: ['value', null],
         executor: ['value', null],
@@ -278,7 +276,6 @@ describe('web-server', () => {
         filesPromise: ['factory', m.createFilesPromise],
         serveStaticFile: ['factory', m.createServeStaticFile],
         serveFile: ['factory', m.createServeFile],
-        readFilePromise: ['factory', m.createReadFilePromise],
         capturedBrowsers: ['value', null],
         reporter: ['value', null],
         executor: ['value', null],
