Skip to content

Commit

Permalink
Run Flow for each renderer separately (#12846)
Browse files Browse the repository at this point in the history
* Generate Flow config on install

We'll need to do pre-renderer Flow passes with different configs.
This is the first step to get it working. We only want the original version checked in.

* Create multiple Flow configs from a template

* Run Flow per renderer

* Lint

* Revert the environment consolidation

I thought this would be a bit cleaner at first because we now have non-environment files in this directory.
But Sebastian is changing these files at the same time so I want to avoid conflicts and keep the PR more tightly scoped. Undo.

* Misc
  • Loading branch information
gaearon authored May 18, 2018
1 parent 40ea053 commit 40addbd
Show file tree
Hide file tree
Showing 8 changed files with 147 additions and 47 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
.DS_STORE
node_modules
scripts/flow/*/.flowconfig
*~
*.pyc
.grunt
Expand All @@ -21,4 +22,4 @@ chrome-user-data
*.iml
.vscode
*.swp
*.swo
*.swo
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@
"linc": "node ./scripts/tasks/linc.js",
"lint": "node ./scripts/tasks/eslint.js",
"lint-build": "node ./scripts/rollup/validate/index.js",
"postinstall": "node node_modules/fbjs-scripts/node/check-dev-engines.js package.json",
"postinstall": "node node_modules/fbjs-scripts/node/check-dev-engines.js package.json && node ./scripts/flow/createFlowConfigs.js",
"debug-test": "cross-env NODE_ENV=development node --inspect-brk node_modules/.bin/jest --config ./scripts/jest/config.source.js --runInBand",
"test": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source.js",
"test-prod": "cross-env NODE_ENV=production jest --config ./scripts/jest/config.source.js",
Expand Down
31 changes: 16 additions & 15 deletions .flowconfig → scripts/flow/config/flowconfig
Original file line number Diff line number Diff line change
@@ -1,25 +1,26 @@
[ignore]

<PROJECT_ROOT>/fixtures/.*
<PROJECT_ROOT>/build/.*
<PROJECT_ROOT>/scripts/bench/.*
.*/scripts/bench/.*

# These shims are copied into external projects:
<PROJECT_ROOT>/scripts/rollup/shims/facebook-www/.*
<PROJECT_ROOT>/scripts/rollup/shims/react-native/.*
.*/rollup/shims/facebook-www/.*
.*/rollup/shims/react-native/.*

<PROJECT_ROOT>/.*/node_modules/y18n/.*
<PROJECT_ROOT>/node_modules/chrome-devtools-frontend/.*
<PROJECT_ROOT>/node_modules/devtools-timeline-model/.*
<PROJECT_ROOT>/node_modules/create-react-class/.*
<PROJECT_ROOT>/.*/__mocks__/.*
<PROJECT_ROOT>/.*/__tests__/.*
.*/node_modules/y18n/.*
.*/node_modules/chrome-devtools-frontend/.*
.*/node_modules/devtools-timeline-model/.*
.*/node_modules/create-react-class/.*
.*/__mocks__/.*
.*/__tests__/.*

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

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

[lints]
untyped-type-import=error
Expand All @@ -42,4 +43,4 @@ suppress_comment=\\(.\\|\n\\)*\\$FlowFixedInNextDeploy
suppress_comment=\\(.\\|\n\\)*\\$FlowExpectedError

[version]
^0.61.0
^0.61.0
44 changes: 44 additions & 0 deletions scripts/flow/createFlowConfigs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
'use strict';

const chalk = require('chalk');
const fs = require('fs');
const mkdirp = require('mkdirp');
const {typedRenderers} = require('./typedRenderers');

const config = fs.readFileSync(__dirname + '/config/flowconfig');

function writeConfig(folder) {
mkdirp.sync(folder);

const disclaimer = `
# ---------------------------------------------------------------#
# NOTE: this file is generated. #
# If you want to edit it, open ./scripts/flow/config/flowconfig. #
# Then run Yarn for changes to take effect. #
# ---------------------------------------------------------------#
`.trim();

const configFile = folder + '/.flowconfig';
let oldConfig;
try {
oldConfig = fs.readFileSync(configFile).toString();
} catch (err) {
oldConfig = null;
}
const newConfig = `
${disclaimer}
${config}
${disclaimer}
`.trim();

if (newConfig !== oldConfig) {
fs.writeFileSync(configFile, newConfig);
console.log(chalk.dim('Wrote a Flow config to ' + configFile));
}
}

// Write multiple configs in different folders
// so that we can run those checks in parallel if we want.
typedRenderers.forEach(renderer => {
writeConfig(__dirname + '/' + renderer);
});
37 changes: 37 additions & 0 deletions scripts/flow/runFlow.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
'use strict';

const chalk = require('chalk');
const spawn = require('child_process').spawn;
const extension = process.platform === 'win32' ? '.cmd' : '';

require('./createFlowConfigs');

async function runFlow(renderer, args) {
return new Promise(resolve => {
console.log(
'Running Flow for the ' + chalk.cyan(renderer) + ' renderer...',
);
spawn('../../../node_modules/.bin/flow' + extension, 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(
'Flow failed for the ' + chalk.red(renderer) + ' renderer',
);
console.log();
process.exit(code);
} else {
console.log(
'Flow passed for the ' + chalk.green(renderer) + ' renderer',
);
console.log();
resolve();
}
});
});
}

module.exports = runFlow;
3 changes: 3 additions & 0 deletions scripts/flow/typedRenderers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
'use strict';

exports.typedRenderers = ['dom', 'fabric', 'native', 'test'];
27 changes: 11 additions & 16 deletions scripts/tasks/flow-ci.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,18 @@

'use strict';

const path = require('path');
const spawn = require('child_process').spawn;

const extension = process.platform === 'win32' ? '.cmd' : '';
process.on('unhandledRejection', err => {
throw err;
});

// This script forces a complete check.
// Use it for the CI.
const runFlow = require('../flow/runFlow');
const {typedRenderers} = require('../flow/typedRenderers');

spawn(path.join('node_modules', '.bin', 'flow' + extension), ['check', '.'], {
// Allow colors to pass through
stdio: 'inherit',
}).on('close', function(code) {
if (code !== 0) {
console.error('Flow failed');
} else {
console.log('Flow passed');
async function checkAll() {
// eslint-disable-next-line no-for-of-loops/no-for-of-loops
for (let renderer of typedRenderers) {
await runFlow(renderer, ['check']);
}
}

process.exit(code);
});
checkAll();
47 changes: 33 additions & 14 deletions scripts/tasks/flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,42 @@

'use strict';

const path = require('path');
const spawn = require('child_process').spawn;
process.on('unhandledRejection', err => {
throw err;
});

const extension = process.platform === 'win32' ? '.cmd' : '';
const chalk = require('chalk');
const runFlow = require('../flow/runFlow');
const {typedRenderers} = require('../flow/typedRenderers');

// This script is using `flow status` for a quick check with a server.
// Use it for local development.

spawn(path.join('node_modules', '.bin', 'flow' + extension), ['status', '.'], {
// Allow colors to pass through
stdio: 'inherit',
}).on('close', function(code) {
if (code !== 0) {
console.error('Flow failed');
} else {
console.log('Flow passed');
}
const primaryRenderer = process.argv[2];
if (typedRenderers.indexOf(primaryRenderer) === -1) {
console.log(
'The ' +
chalk.red('yarn flow') +
' command now requires you to pick a primary renderer:'
);
console.log();
typedRenderers.forEach(renderer => {
console.log(' * ' + chalk.cyan('yarn flow ' + renderer));
});
console.log();
console.log('If you are not sure, run ' + chalk.green('yarn flow dom') + '.');
console.log(
'This will still typecheck non-DOM packages, although less precisely.'
);
console.log();
console.log('Note that checks for all renderers will run on CI.');
console.log(
'You can also do this locally with ' +
chalk.cyan('yarn flow-ci') +
' but it will be slow.'
);
console.log();
process.exit(1);
}

process.exit(code);
});
runFlow(primaryRenderer, ['status']);

0 comments on commit 40addbd

Please sign in to comment.