From 5f13e11784c283872c97501b9e21402987429c56 Mon Sep 17 00:00:00 2001 From: johnjbarton Date: Mon, 26 Aug 2019 10:41:32 -0700 Subject: [PATCH] fix(server): Simplify 'dom' inclusion. (#3356) The karma preprocessor caches file content: no need to read it async in the server loop. --- lib/middleware/karma.js | 17 +--------------- lib/web-server.js | 22 +-------------------- test/unit/middleware/karma.spec.js | 31 +++++------------------------- test/unit/web-server.spec.js | 3 --- 4 files changed, 7 insertions(+), 66 deletions(-) diff --git a/lib/middleware/karma.js b/lib/middleware/karma.js index 4bcb98f80..dd1ee1d00 100644 --- 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(``) } else if (fileType === 'dom') { - scriptTags.push(includedContent.get(file.path)) + scriptTags.push(file.content) } else if (fileType === 'html') { scriptTags.push(``) } 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 index 6bf76ebbb..3f271d3bc 100644 --- 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 index 580d5f006..c7c474eaf 100644 --- 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 index efd8b3ae4..5d9f4f18e 100644 --- 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],