Skip to content

Commit

Permalink
Make Flow work with your editor (#18664)
Browse files Browse the repository at this point in the history
We typecheck the reconciler against each one of our host configs.
`yarn flow dom` checks it against the DOM renderer, `yarn flow native`
checks it against the native renderer, and so on.

To do this, we generate separate flowconfig files.

Currently, there is no root-level host config, so running Flow
directly via `flow` CLI doesn't work. You have to use the `yarn flow`
command and pick a specific renderer.

A drawback of this design, though, is that our Flow setup doesn't work
with other tooling. Namely, editor integrations.

I think the intent of this was maybe so you don't run Flow against a
renderer than you intended, see it pass, and wrongly think you fixed
all the errors. However, since they all run in CI, I don't think this
is a big deal. In practice, I nearly always run Flow against the same
renderer (DOM), and I'm guessing that's the most common workflow for
others, too.

So what I've done in this commit is modify the `yarn flow` command to
copy the generated `.flowconfig` file into the root directory. The
editor integration will pick this up and show Flow information for
whatever was the last renderer you checked.

Everything else about the setup is the same, and all the renderers will
continue to be checked by CI.
  • Loading branch information
acdlite authored Apr 18, 2020
1 parent cfefc81 commit 52a0d6b
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 14 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
.DS_STORE
node_modules
scripts/flow/*/.flowconfig
.flowconfig
*~
*.pyc
.grunt
Expand Down
17 changes: 9 additions & 8 deletions scripts/flow/config/flowconfig
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[ignore]
.*/scripts/bench/.*
.*/build/.*

# These shims are copied into external projects:
.*/rollup/shims/facebook-www/.*
Expand All @@ -20,16 +21,16 @@
%REACT_RENDERER_FLOW_IGNORES%

[include]
../../../node_modules/
../../../packages/
../../../scripts/
./node_modules/
./packages/
./scripts/

[libs]
../../../node_modules/fbjs/flow/lib/dev.js
../environment.js
../react-devtools.js
../react-native-host-hooks.js
../react-relay-hooks.js
./node_modules/fbjs/flow/lib/dev.js
./scripts/flow/environment.js
./scripts/flow/react-devtools.js
./scripts/flow/react-native-host-hooks.js
./scripts/flow/react-relay-hooks.js

[lints]
untyped-type-import=error
Expand Down
53 changes: 47 additions & 6 deletions scripts/flow/runFlow.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,65 @@
'use strict';

const chalk = require('chalk');
const spawn = require('child_process').spawn;
const {spawn} = require('child_process');
const fs = require('fs');

// TODO: This generates all the renderer configs at once. Originally this was
// to allow the possibility of running multiple Flow processes in parallel, but
// that never happened. If we did, we'd probably do this in CI, anyway, and run
// on multiple machines. So instead we could remove this intermediate step and
// generate only the config for the specified renderer.
require('./createFlowConfigs');

async function runFlow(renderer, args) {
return new Promise(resolve => {
console.log(
'Running Flow on the ' + chalk.yellow(renderer) + ' renderer...',
);
let cmd = __dirname + '/../../node_modules/.bin/flow';
if (process.platform === 'win32') {
cmd = cmd.replace(/\//g, '\\') + '.cmd';
}

// Copy renderer flowconfig file to the root of the project so that it
// works with editor integrations. This means that the Flow config used by
// the editor will correspond to the last renderer you checked.
const srcPath =
process.cwd() + '/scripts/flow/' + renderer + '/.flowconfig';
const srcStat = fs.statSync(__dirname + '/config/flowconfig');
const destPath = './.flowconfig';
if (fs.existsSync(destPath)) {
const oldConfig = fs.readFileSync(destPath) + '';
const newConfig = fs.readFileSync(srcPath) + '';
if (oldConfig !== newConfig) {
// Use the mtime to detect if the file was manually edited. If so,
// log an error.
const destStat = fs.statSync(destPath);
if (destStat.mtimeMs - srcStat.mtimeMs > 1) {
console.error(
chalk.red(
'Detected manual changes to .flowconfig, which is a generated ' +
'file. These changes have been discarded.\n\n' +
'To change the Flow config, edit the template in ' +
'scripts/flow/config/flowconfig. Then run this command again.\n',
),
);
}
fs.unlinkSync(destPath);
fs.copyFileSync(srcPath, destPath);
// Set the mtime of the copied file to be same as the original file,
// so that the ahove check works.
fs.utimesSync(destPath, srcStat.atime, srcStat.mtime);
}
} else {
fs.copyFileSync(srcPath, destPath);
fs.utimesSync(destPath, srcStat.atime, srcStat.mtime);
}

console.log(
'Running Flow on the ' + chalk.yellow(renderer) + ' renderer...',
);

spawn(cmd, args, {
// Allow colors to pass through:
stdio: 'inherit',
// Use a specific renderer config:
cwd: process.cwd() + '/scripts/flow/' + renderer + '/',
}).on('close', function(code) {
if (code !== 0) {
console.error(
Expand Down

0 comments on commit 52a0d6b

Please sign in to comment.