From 6e1ec16e30b9510b187568a09050ddc5433cad04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Jim=C3=A9nez=20Es=C3=BAn?= Date: Tue, 10 Jul 2018 16:48:05 +0100 Subject: [PATCH] Avoid reading files from Haste map if not needed (#6667) --- CHANGELOG.md | 1 + .../src/__tests__/index.test.js | 5 ++ .../src/__tests__/worker.test.js | 58 +++++++++++++++++-- packages/jest-haste-map/src/index.js | 8 +++ packages/jest-haste-map/src/types.js | 1 + packages/jest-haste-map/src/worker.js | 38 ++++++------ 6 files changed, 87 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41dd1048a71b..a5b167ace3cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ### Features +- `[jest-haste-map]` Add `computeDependencies` flag to avoid opening files if not needed ([#6667](https://github.com/facebook/jest/pull/6667)) - `[jest-runtime]` Support `require.resolve.paths` ([#6471](https://github.com/facebook/jest/pull/6471)) - `[jest-runtime]` Support `paths` option for `require.resolve` ([#6471](https://github.com/facebook/jest/pull/6471)) diff --git a/packages/jest-haste-map/src/__tests__/index.test.js b/packages/jest-haste-map/src/__tests__/index.test.js index bbfca273573d..4bb61a42bd47 100644 --- a/packages/jest-haste-map/src/__tests__/index.test.js +++ b/packages/jest-haste-map/src/__tests__/index.test.js @@ -899,6 +899,7 @@ describe('HasteMap', () => { expect(mockWorker.mock.calls).toEqual([ [ { + computeDependencies: true, computeSha1: false, filePath: '/fruits/__mocks__/Pear.js', hasteImplModulePath: undefined, @@ -906,6 +907,7 @@ describe('HasteMap', () => { ], [ { + computeDependencies: true, computeSha1: false, filePath: '/fruits/banana.js', hasteImplModulePath: undefined, @@ -913,6 +915,7 @@ describe('HasteMap', () => { ], [ { + computeDependencies: true, computeSha1: false, filePath: '/fruits/pear.js', hasteImplModulePath: undefined, @@ -920,6 +923,7 @@ describe('HasteMap', () => { ], [ { + computeDependencies: true, computeSha1: false, filePath: '/fruits/strawberry.js', hasteImplModulePath: undefined, @@ -927,6 +931,7 @@ describe('HasteMap', () => { ], [ { + computeDependencies: true, computeSha1: false, filePath: '/vegetables/melon.js', hasteImplModulePath: undefined, diff --git a/packages/jest-haste-map/src/__tests__/worker.test.js b/packages/jest-haste-map/src/__tests__/worker.test.js index 13406384eb7b..e2637d9860aa 100644 --- a/packages/jest-haste-map/src/__tests__/worker.test.js +++ b/packages/jest-haste-map/src/__tests__/worker.test.js @@ -18,12 +18,14 @@ const {worker, getSha1} = require('../worker'); let mockFs; let readFileSync; +let readFile; describe('worker', () => { ConditionalTest.skipSuiteOnWindows(); beforeEach(() => { mockFs = { + '/fruits/apple.png': Buffer.from([137, 80, 78, 71, 13, 10, 26, 10]), '/fruits/banana.js': [ '/**', ' * @providesModule Banana', @@ -42,10 +44,17 @@ describe('worker', () => { ' * @providesModule Strawberry', ' */', ].join('\n'), - '/package.json': ['{', ' "name": "haste-package"', '}'].join('\n'), + '/package.json': [ + '{', + ' "name": "haste-package",', + ' "main": "foo.js"', + '}', + ].join('\n'), }; readFileSync = fs.readFileSync; + readFile = fs.readFile; + fs.readFileSync = jest.fn((path, options) => { if (mockFs[path]) { return options === 'utf8' ? mockFs[path] : Buffer.from(mockFs[path]); @@ -53,20 +62,30 @@ describe('worker', () => { throw new Error(`Cannot read path '${path}'.`); }); + + fs.readFile = jest.fn(readFile); }); afterEach(() => { fs.readFileSync = readFileSync; + fs.readFile = readFile; }); it('parses JavaScript files and extracts module information', async () => { - expect(await worker({filePath: '/fruits/pear.js'})).toEqual({ + expect( + await worker({computeDependencies: true, filePath: '/fruits/pear.js'}), + ).toEqual({ dependencies: ['Banana', 'Strawberry'], id: 'Pear', module: ['/fruits/pear.js', H.MODULE], }); - expect(await worker({filePath: '/fruits/strawberry.js'})).toEqual({ + expect( + await worker({ + computeDependencies: true, + filePath: '/fruits/strawberry.js', + }), + ).toEqual({ dependencies: [], id: 'Strawberry', module: ['/fruits/strawberry.js', H.MODULE], @@ -75,6 +94,7 @@ describe('worker', () => { it('delegates to hasteImplModulePath for getting the id', async () => { const moduleData = await worker({ + computeDependencies: true, filePath: '/fruits/strawberry.js', hasteImplModulePath: path.resolve(__dirname, 'haste_impl.js'), }); @@ -90,7 +110,9 @@ describe('worker', () => { }); it('parses package.json files as haste packages', async () => { - expect(await worker({filePath: '/package.json'})).toEqual({ + expect( + await worker({computeDependencies: true, filePath: '/package.json'}), + ).toEqual({ dependencies: undefined, id: 'haste-package', module: ['/package.json', H.PACKAGE], @@ -99,8 +121,9 @@ describe('worker', () => { it('returns an error when a file cannot be accessed', async () => { let error = null; + try { - await worker({filePath: '/kiwi.js'}); + await worker({computeDependencies: true, filePath: '/kiwi.js'}); } catch (err) { error = err; } @@ -108,7 +131,11 @@ describe('worker', () => { expect(error.message).toEqual(`Cannot read path '/kiwi.js'.`); }); - it('simply computes SHA-1s when requested', async () => { + it('simply computes SHA-1s when requested (works well with binary data)', async () => { + expect( + await getSha1({computeSha1: true, filePath: '/fruits/apple.png'}), + ).toEqual({sha1: '4caece539b039b16e16206ea2478f8c5ffb2ca05'}); + expect( await getSha1({computeSha1: false, filePath: '/fruits/banana.js'}), ).toEqual({sha1: null}); @@ -125,4 +152,23 @@ describe('worker', () => { getSha1({computeSha1: true, filePath: '/i/dont/exist.js'}), ).rejects.toThrow(); }); + + it('avoids computing dependencies if not requested and Haste does not need it', async () => { + expect( + await worker({ + computeDependencies: false, + filePath: '/fruits/pear.js', + hasteImplModulePath: path.resolve(__dirname, 'haste_impl.js'), + }), + ).toEqual({ + dependencies: undefined, + id: 'pear', + module: ['/fruits/pear.js', H.MODULE], + sha1: undefined, + }); + + // Ensure not disk access happened. + expect(fs.readFileSync).not.toHaveBeenCalled(); + expect(fs.readFile).not.toHaveBeenCalled(); + }); }); diff --git a/packages/jest-haste-map/src/index.js b/packages/jest-haste-map/src/index.js index 161fd4970a10..3da0ae655621 100644 --- a/packages/jest-haste-map/src/index.js +++ b/packages/jest-haste-map/src/index.js @@ -44,6 +44,7 @@ type HType = typeof H; type Options = { cacheDirectory?: string, + computeDependencies?: boolean, computeSha1?: boolean, console?: Console, extensions: Array, @@ -65,6 +66,7 @@ type Options = { type InternalOptions = { cacheDirectory: string, + computeDependencies: boolean, computeSha1: boolean, extensions: Array, forceNodeFilesystemAPI: boolean, @@ -213,6 +215,10 @@ class HasteMap extends EventEmitter { super(); this._options = { cacheDirectory: options.cacheDirectory || os.tmpdir(), + computeDependencies: + options.computeDependencies === undefined + ? true + : options.computeDependencies, computeSha1: options.computeSha1 || false, extensions: options.extensions, forceNodeFilesystemAPI: !!options.forceNodeFilesystemAPI, @@ -454,6 +460,7 @@ class HasteMap extends EventEmitter { if (computeSha1) { return this._getWorker(workerOptions) .getSha1({ + computeDependencies: this._options.computeDependencies, computeSha1, filePath, hasteImplModulePath: this._options.hasteImplModulePath, @@ -510,6 +517,7 @@ class HasteMap extends EventEmitter { return this._getWorker(workerOptions) .worker({ + computeDependencies: this._options.computeDependencies, computeSha1, filePath, hasteImplModulePath: this._options.hasteImplModulePath, diff --git a/packages/jest-haste-map/src/types.js b/packages/jest-haste-map/src/types.js index aaedde6a83e7..18db89daf8ba 100644 --- a/packages/jest-haste-map/src/types.js +++ b/packages/jest-haste-map/src/types.js @@ -12,6 +12,7 @@ import type {InternalHasteMap, ModuleMetaData} from 'types/HasteMap'; export type IgnoreMatcher = (item: string) => boolean; export type WorkerMessage = { + computeDependencies: boolean, computeSha1: boolean, filePath: string, hasteImplModulePath?: string, diff --git a/packages/jest-haste-map/src/worker.js b/packages/jest-haste-map/src/worker.js index cf58db7afb48..7a60fc965bcc 100644 --- a/packages/jest-haste-map/src/worker.js +++ b/packages/jest-haste-map/src/worker.js @@ -22,7 +22,7 @@ const PACKAGE_JSON = path.sep + 'package.json'; let hasteImpl: ?HasteImpl = null; let hasteImplModulePath: ?string = null; -function computeSha1(content) { +function sha1hex(content: string | Buffer): string { return crypto .createHash('sha1') .update(content) @@ -42,18 +42,25 @@ export async function worker(data: WorkerMessage): Promise { hasteImpl = (require(hasteImplModulePath): HasteImpl); } - const filePath = data.filePath; let content; let dependencies; let id; let module; let sha1; - // Process a package.json that is returned as a PACKAGE type with its name. + const {computeDependencies, computeSha1, filePath} = data; + const getContent = (): string => { + if (content === undefined) { + content = fs.readFileSync(filePath, 'utf8'); + } + + return content; + }; + if (filePath.endsWith(PACKAGE_JSON)) { - content = fs.readFileSync(filePath, 'utf8'); + // Process a package.json that is returned as a PACKAGE type with its name. try { - const fileData = JSON.parse(content); + const fileData = JSON.parse(getContent()); if (fileData.name) { id = fileData.name; @@ -62,19 +69,18 @@ export async function worker(data: WorkerMessage): Promise { } catch (err) { throw new Error(`Cannot parse ${filePath} as JSON: ${err.message}`); } - - // Process a randome file that is returned as a MODULE. } else if (!blacklist.has(filePath.substr(filePath.lastIndexOf('.')))) { - content = fs.readFileSync(filePath, 'utf8'); - + // Process a random file that is returned as a MODULE. if (hasteImpl) { id = hasteImpl.getHasteName(filePath); } else { - const doc = docblock.parse(docblock.extract(content)); + const doc = docblock.parse(docblock.extract(getContent())); id = [].concat(doc.providesModule || doc.provides)[0]; } - dependencies = extractRequires(content); + if (computeDependencies) { + dependencies = extractRequires(getContent()); + } if (id) { module = [filePath, H.MODULE]; @@ -82,12 +88,8 @@ export async function worker(data: WorkerMessage): Promise { } // If a SHA-1 is requested on update, compute it. - if (data.computeSha1) { - if (content == null) { - content = fs.readFileSync(filePath); - } - - sha1 = computeSha1(content); + if (computeSha1) { + sha1 = sha1hex(getContent() || fs.readFileSync(filePath)); } return {dependencies, id, module, sha1}; @@ -95,7 +97,7 @@ export async function worker(data: WorkerMessage): Promise { export async function getSha1(data: WorkerMessage): Promise { const sha1 = data.computeSha1 - ? computeSha1(fs.readFileSync(data.filePath)) + ? sha1hex(fs.readFileSync(data.filePath)) : null; return {