Skip to content

Commit

Permalink
don’t use watchman when not watching
Browse files Browse the repository at this point in the history
Summary:
Fixes #1644

This PR will update relay-compiler to not use watchman unless watching files.
Closes #1966

Reviewed By: asiandrummer

Differential Revision: D5522984

Pulled By: leebyron

fbshipit-source-id: 5c18f9e03691a542d9c77b9c1858a734ed6b6792
  • Loading branch information
robrichard authored and facebook-github-bot committed Aug 2, 2017
1 parent 06ec89b commit 4f80e02
Show file tree
Hide file tree
Showing 9 changed files with 608 additions and 178 deletions.
1 change: 1 addition & 0 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const babelOptions = require('./scripts/getBabelOptions')({
'crypto': 'crypto',
'fb-watchman': 'fb-watchman',
'fs': 'fs',
'fast-glob': 'fast-glob',
'graphql': 'graphql',
'immutable': 'immutable',
'net': 'net',
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"eslint-plugin-flowtype": "2.30.4",
"eslint-plugin-react": "5.2.2",
"event-stream": "3.3.4",
"fast-glob": "^1.0.1",
"fb-watchman": "2.0.0",
"fbjs": "0.8.14",
"fbjs-scripts": "0.7.1",
Expand Down
1 change: 1 addition & 0 deletions packages/relay-compiler/RelayCompilerPublic.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const RelayIRTransforms = require('RelayIRTransforms');
const RelayMultiReporter = require('RelayMultiReporter');

export type {CompileResult} from 'RelayCodegenTypes';
export type {ParserConfig, WriterConfig} from 'RelayCodegenRunner';

module.exports = {
Compiler: RelayCompiler,
Expand Down
37 changes: 36 additions & 1 deletion packages/relay-compiler/bin/RelayCompilerBin.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const RelayConsoleReporter = require('RelayConsoleReporter');
const RelayFileIRParser = require('RelayFileIRParser');
const RelayFileWriter = require('RelayFileWriter');
const RelayIRTransforms = require('RelayIRTransforms');
const RelayWatchmanClient = require('RelayWatchmanClient');

const formatGeneratedModule = require('formatGeneratedModule');
const fs = require('fs');
Expand Down Expand Up @@ -60,6 +61,27 @@ function buildWatchExpression(options: {
];
}

function getFilepathsFromGlob(
baseDir,
options: {
extensions: Array<string>,
include: Array<string>,
exclude: Array<string>,
},
): Array<string> {
const {extensions, include, exclude} = options;
const patterns = include.map(inc => `${inc}/*.+(${extensions.join('|')})`);

// $FlowFixMe
const glob = require('fast-glob');
return glob.sync(patterns, {
cwd: baseDir,
bashNative: [],

This comment has been minimized.

Copy link
@edvinerikson

edvinerikson Aug 30, 2017

Contributor

What's the reason to not use the default value here? This seems to be causing the issues in #2042

cc @robrichard

This comment has been minimized.

Copy link
@robrichard

robrichard Aug 30, 2017

Author Contributor

I'm not able to reproduce the issue in #2042. I ran into some issues using the default value - I think the globs were not matching as expected.

onlyFiles: true,
ignore: exclude,
});
}

/* eslint-disable no-console-disallow */

async function run(options: {
Expand All @@ -69,6 +91,7 @@ async function run(options: {
include: Array<string>,
exclude: Array<string>,
verbose: boolean,
watchman: boolean,
watch?: ?boolean,
}) {
const schemaPath = path.resolve(process.cwd(), options.schema);
Expand All @@ -79,6 +102,9 @@ async function run(options: {
if (!fs.existsSync(srcDir)) {
throw new Error(`--source path does not exist: ${srcDir}.`);
}
if (options.watch && !options.watchman) {
throw new Error('Watchman is required to watch for changes.');
}
if (options.watch && !hasWatchmanRootFile(srcDir)) {
throw new Error(
`
Expand All @@ -96,13 +122,17 @@ Ensure that one such file exists in ${srcDir} or its parents.

const reporter = new RelayConsoleReporter({verbose: options.verbose});

const useWatchman =
options.watchman && (await RelayWatchmanClient.isAvailable());

const parserConfigs = {
default: {
baseDir: srcDir,
getFileFilter: RelayFileIRParser.getFileFilter,
getParser: RelayFileIRParser.getParser,
getSchema: () => getSchema(schemaPath),
watchmanExpression: buildWatchExpression(options),
watchmanExpression: useWatchman ? buildWatchExpression(options) : null,
filepaths: useWatchman ? null : getFilepathsFromGlob(srcDir, options),
},
};
const writerConfigs = {
Expand Down Expand Up @@ -234,6 +264,11 @@ const argv = yargs
describe: 'More verbose logging',
type: 'boolean',
},
watchman: {
describe: 'Use watchman when not in watch mode',
type: 'boolean',
default: true,
},
watch: {
describe: 'If specified, watches files and regenerates on changes',
type: 'boolean',
Expand Down
41 changes: 33 additions & 8 deletions packages/relay-compiler/generic/codegen/RelayCodegenRunner.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ export type ParserConfig = {|
getFileFilter?: (baseDir: string) => FileFilter,
getParser: (baseDir: string) => FileParser,
getSchema: () => GraphQLSchema,
watchmanExpression: WatchmanExpression,
watchmanExpression?: ?WatchmanExpression,
filepaths?: ?Array<string>,
|};

type ParserConfigs = {
Expand Down Expand Up @@ -193,14 +194,34 @@ class RelayCodegenRunner {

const parserConfig = this.parserConfigs[parserName];
this.parsers[parserName] = parserConfig.getParser(parserConfig.baseDir);
const filter = parserConfig.getFileFilter
? parserConfig.getFileFilter(parserConfig.baseDir)
: anyFileFilter;

const files = await RelayCodegenWatcher.queryFiles(
parserConfig.baseDir,
parserConfig.watchmanExpression,
parserConfig.getFileFilter
? parserConfig.getFileFilter(parserConfig.baseDir)
: anyFileFilter,
);
if (parserConfig.filepaths && parserConfig.watchmanExpression) {
throw new Error(
'Provide either `watchmanExpression` or `filepaths` but not both.',
);
}

let files;
if (parserConfig.watchmanExpression) {
files = await RelayCodegenWatcher.queryFiles(
parserConfig.baseDir,
parserConfig.watchmanExpression,
filter,
);
} else if (parserConfig.filepaths) {
files = await RelayCodegenWatcher.queryFilepaths(
parserConfig.baseDir,
parserConfig.filepaths,
filter,
);
} else {
throw new Error(
'Either `watchmanExpression` or `filepaths` is required to query files',
);
}
this.parseFileChanges(parserName, files);
}

Expand Down Expand Up @@ -318,6 +339,10 @@ class RelayCodegenRunner {
async watch(parserName: string): Promise<void> {
const parserConfig = this.parserConfigs[parserName];

if (!parserConfig.watchmanExpression) {
throw new Error('`watchmanExpression` is required to watch files');
}

// watchCompile starts with a full set of files as the changes
// But as we need to set everything up due to potential parser dependencies,
// we should prevent the first watch callback from doing anything.
Expand Down
17 changes: 17 additions & 0 deletions packages/relay-compiler/generic/codegen/RelayCodegenWatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,22 @@ async function getFields(client: RelayWatchmanClient): Promise<Array<string>> {
return fields;
}

// For use when not using Watchman.
async function queryFilepaths(
baseDir: string,
filepaths: Array<string>,
filter: FileFilter,
): Promise<Set<File>> {
// Construct WatchmanChange objects as an intermediate step before
// calling updateFiles to produce file content.
const files = filepaths.map(filepath => ({
name: filepath,
exists: true,
'content.sha1hex': null,
}));
return updateFiles(new Set(), baseDir, filter, files);
}

/**
* Provides a simplified API to the watchman API.
* Given some base directory and a list of subdirectories it calls the callback
Expand Down Expand Up @@ -192,6 +208,7 @@ function hashFile(filename: string): string {

module.exports = {
queryFiles,
queryFilepaths,
watch,
watchFiles,
watchCompile,
Expand Down
8 changes: 8 additions & 0 deletions packages/relay-compiler/generic/core/RelayWatchmanClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ const watchman = require('fb-watchman');
class RelayWatchmanClient {
_client: any;

static isAvailable(): Promise<boolean> {
return new Promise(resolve => {
const client = new RelayWatchmanClient();
client.on('error', () => resolve(false));
client.hasCapability('relative_root').then(resolve, () => resolve(false));
});
}

constructor() {
this._client = new watchman.Client();
}
Expand Down
1 change: 1 addition & 0 deletions packages/relay-compiler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"babel-types": "6.25.0",
"babylon": "6.17.3",
"chalk": "^1.1.3",
"fast-glob": "^1.0.1",
"fb-watchman": "^2.0.0",
"fbjs": "^0.8.1",
"graphql": "^0.10.5",
Expand Down
Loading

0 comments on commit 4f80e02

Please sign in to comment.