Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid reading files from Haste map if not needed #6667

Merged
merged 2 commits into from
Jul 10, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
5 changes: 5 additions & 0 deletions packages/jest-haste-map/src/__tests__/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -899,34 +899,39 @@ describe('HasteMap', () => {
expect(mockWorker.mock.calls).toEqual([
[
{
computeDependencies: true,
computeSha1: false,
filePath: '/fruits/__mocks__/Pear.js',
hasteImplModulePath: undefined,
},
],
[
{
computeDependencies: true,
computeSha1: false,
filePath: '/fruits/banana.js',
hasteImplModulePath: undefined,
},
],
[
{
computeDependencies: true,
computeSha1: false,
filePath: '/fruits/pear.js',
hasteImplModulePath: undefined,
},
],
[
{
computeDependencies: true,
computeSha1: false,
filePath: '/fruits/strawberry.js',
hasteImplModulePath: undefined,
},
],
[
{
computeDependencies: true,
computeSha1: false,
filePath: '/vegetables/melon.js',
hasteImplModulePath: undefined,
Expand Down
58 changes: 52 additions & 6 deletions packages/jest-haste-map/src/__tests__/worker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -42,31 +44,48 @@ 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]);
}

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],
Expand All @@ -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'),
});
Expand All @@ -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],
Expand All @@ -99,16 +121,21 @@ 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;
}

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});
Expand All @@ -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();
});
});
8 changes: 8 additions & 0 deletions packages/jest-haste-map/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type HType = typeof H;

type Options = {
cacheDirectory?: string,
computeDependencies?: boolean,
computeSha1?: boolean,
console?: Console,
extensions: Array<string>,
Expand All @@ -65,6 +66,7 @@ type Options = {

type InternalOptions = {
cacheDirectory: string,
computeDependencies: boolean,
computeSha1: boolean,
extensions: Array<string>,
forceNodeFilesystemAPI: boolean,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -510,6 +517,7 @@ class HasteMap extends EventEmitter {

return this._getWorker(workerOptions)
.worker({
computeDependencies: this._options.computeDependencies,
computeSha1,
filePath,
hasteImplModulePath: this._options.hasteImplModulePath,
Expand Down
1 change: 1 addition & 0 deletions packages/jest-haste-map/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
38 changes: 20 additions & 18 deletions packages/jest-haste-map/src/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -42,18 +42,25 @@ export async function worker(data: WorkerMessage): Promise<WorkerMetadata> {
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;
Expand All @@ -62,40 +69,35 @@ export async function worker(data: WorkerMessage): Promise<WorkerMetadata> {
} 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];
}
}

// 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};
}

export async function getSha1(data: WorkerMessage): Promise<WorkerMetadata> {
const sha1 = data.computeSha1
? computeSha1(fs.readFileSync(data.filePath))
? sha1hex(fs.readFileSync(data.filePath))
: null;

return {
Expand Down