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

Add flow typechecking as a webpack plugin #1152

Closed
wants to merge 75 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
75 commits
Select commit Hold shift + click to select a range
10ec304
Setup a plugin to run a flow checking lifecycle
rricard Dec 4, 2016
c7eacf8
Make flow fail on CI builds
rricard Dec 4, 2016
b0c39e7
Run flow as a server and check with status
rricard Dec 4, 2016
de16269
Add flow ignore comment to the start hints
rricard Dec 4, 2016
31b2e04
Add flow-typed resolution at start
rricard Dec 4, 2016
d4763a5
Reset server stderr on restart
rricard Dec 4, 2016
2f34a67
Move options as explicit props of the plugin
rricard Dec 4, 2016
79b3b86
Use arrow functions
rricard Dec 4, 2016
d004f2d
Refactor using promises and no class attrs
rricard Dec 4, 2016
ec8d230
Add documentation for out-of-the-box flow
rricard Dec 5, 2016
bb39b19
Schedule project init to complete before 1st check
rricard Dec 5, 2016
8878502
Process code-based flow checks
rricard Dec 5, 2016
6a5cfc1
Get flow version internally without config
rricard Dec 5, 2016
513c0a9
Check global flow version
rricard Dec 5, 2016
719ac82
Add flow-typed to gitignore automatically
rricard Dec 5, 2016
c76f15e
Add optional flow to end to end testing
rricard Dec 5, 2016
ba7a05a
Run flow even if init failed
rricard Dec 5, 2016
2362346
Remove fbjs ignores now that it's fixed
rricard Dec 5, 2016
f83fa6a
Remove flow-typed lib import (known by default!)
rricard Dec 5, 2016
f42c5cc
In e2e: assert flow is correctly showing errors
rricard Dec 5, 2016
f9ae1f0
Only change gitignore if we create a flowconfig
rricard Dec 8, 2016
c5e2102
Ensure the init promise is correctly triggered
rricard Dec 9, 2016
e4e2111
Don't create an uncatched promise
rricard Dec 9, 2016
f9bf33d
Don't reinit flow in a child compilation
rricard Dec 9, 2016
4448e98
Remove flow-typed/ dir assertion
rricard Dec 12, 2016
db929a1
Reapply a lost change from #1233 during rebase
rricard Dec 12, 2016
eac318d
Consistent ESLint capitalization
rricard Dec 12, 2016
054357f
Run flow version check before the rest
rricard Dec 12, 2016
b3e5609
Transmit via warnings why Flow was disabled
rricard Dec 12, 2016
cabeadb
Don't fail on tarball react-scripts
rricard Dec 12, 2016
fa6d3c8
Celebrate when the flow tests pass!
rricard Dec 12, 2016
05ae460
Show npm commands in cyan
rricard Dec 12, 2016
e30a959
"Hide" flow err suppression in dev utils
rricard Dec 12, 2016
3ed5f2c
Only highlight the npm word
rricard Dec 12, 2016
026e581
Make flow-typed provision sequential
rricard Dec 13, 2016
6e44124
Redisign & simplify flow e2e tests
rricard Dec 13, 2016
3047218
Correctly ignore invalid comp errors
rricard Dec 13, 2016
09c2c13
Add ref to an issue comment on the silencing
rricard Dec 13, 2016
3949fb3
Remove debugging command in e2e.sh
rricard Dec 13, 2016
159638b
Remove planning to add the feature in doc[ci skip]
rricard Dec 13, 2016
5105c29
No build test for flow (duplicate)
rricard Dec 13, 2016
4fa4569
Simpler second flow test
rricard Dec 13, 2016
ba483c6
After eject, the .flowconfig should stay
rricard Dec 14, 2016
a48e8c3
Use Flow 0.37.0
rricard Dec 14, 2016
9e1957e
Directly target the flow-typed executable
rricard Dec 14, 2016
c834095
Revert "Directly target the flow-typed executable"
rricard Dec 14, 2016
f5d3982
Find flow-typed in the npm flat struct
rricard Dec 15, 2016
cfc1115
Define flow-typed path with project root
rricard Dec 15, 2016
030f7c1
Resolve flow-typed via module resolution
rricard Dec 15, 2016
37444f2
Remove unneeded complexity
rricard Dec 15, 2016
08ca783
Make run flow-typed executable
rricard Dec 15, 2016
51cca10
Don't put flow-typed on gitignore
Gregoor Mar 18, 2017
7e33f9d
Merge branch 'master' into flow
rricard Apr 22, 2017
a3a78f7
Appease linter
Timer Apr 22, 2017
eb9e341
Update flow
Timer Apr 22, 2017
137f59f
Add basic flow type checking
Timer Apr 22, 2017
2fd2dbc
Adjust format function
Timer Apr 22, 2017
7c5749d
Remove flow-typed stub
Timer Apr 22, 2017
f4299ef
Use access instead of stat to prevent dual-compile
Timer Apr 23, 2017
a3fb2df
Add some friendly comments
Timer Apr 23, 2017
de99610
Lock down flow version
Timer Apr 23, 2017
29628ef
Update comment logs
Timer Apr 23, 2017
30494dc
Remove old method
Timer Apr 23, 2017
f850266
Remove from e2e
Timer Apr 23, 2017
8d1f515
Check flow version for performance
Timer Apr 23, 2017
1df2cb5
Remove warning prefix for flow messages
Timer Apr 23, 2017
70ad8dd
Update README
Timer Apr 23, 2017
45841cd
Don't double compute
Timer Apr 23, 2017
dab6f9d
Fix linter errors
Timer Apr 23, 2017
1cb808e
Use default perm check
Timer Apr 23, 2017
e94387c
Handle already running server since it exits with a non-zero
Timer Apr 24, 2017
45e6c29
Reduce nesting
Timer Apr 24, 2017
fb4735c
Simulate a mutex for startFlow
Timer Apr 24, 2017
800af94
Fix unhandled rejection
Timer May 4, 2017
ad625f1
Upgrade flow
Timer May 4, 2017
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
220 changes: 220 additions & 0 deletions packages/react-dev-utils/FlowTypecheckPlugin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,220 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/

'use strict';

const fs = require('fs');
const path = require('path');
const chalk = require('chalk');
const childProcess = require('child_process');
const flowBinPath = require('flow-bin');

function exec(command, args, options) {
return new Promise((resolve, reject) => {
var stdout = new Buffer('');
var stderr = new Buffer('');
var oneTimeProcess = childProcess.spawn(command, args, options);
oneTimeProcess.stdout.on('data', chunk => {
stdout = Buffer.concat([stdout, chunk]);
});
oneTimeProcess.stderr.on('data', chunk => {
stderr = Buffer.concat([stderr, chunk]);
});
oneTimeProcess.on('error', error => reject(error));
oneTimeProcess.on('exit', code => {
switch (code) {
case 0: {
return resolve(stdout);
}
default: {
return reject(new Error(Buffer.concat([stdout, stderr]).toString()));
}
}
});
});
}

function createVersionWarning(flowVersion) {
return 'Flow: ' +
chalk.red(
chalk.bold(
`Your global flow version is incompatible with this tool.
To fix warning, uninstall it or run \`npm install -g flow-bin@${flowVersion}\`.`
)
);
}

function formatFlowErrors(error) {
return error
.toString()
.split('\n')
.filter(line => {
return !(/flow is still initializing/.test(line) ||
/Found \d+ error/.test(line) ||
/The flow server is not responding/.test(line) ||
/Going to launch a new one/.test(line) ||
/The flow server is not responding/.test(line) ||
/Spawned flow server/.test(line) ||
/Logs will go to/.test(line) ||
/version didn't match the client's/.test(line));
})
.map(line => line.replace(/^Error:\s*/, ''))
.join('\n');
}

function getFlowVersion(global) {
return exec(global ? 'flow' : flowBinPath, ['version', '--json'])
.then(data => JSON.parse(data.toString('utf8')).semver || '0.0.0')
.catch(() => null);
}

class FlowTypecheckPlugin {
constructor() {
this.shouldRun = false;
this.flowStarted = false;
this.flowStarting = null;

this.flowVersion = require(path.join(
__dirname,
'package.json'
)).dependencies['flow-bin'];
}

startFlow(cwd) {
if (this.flowStarted) {
return Promise.resolve();
}
if (this.flowStarting != null) {
return this.flowStarting.then(err => {
// We need to do it like this because of unhandled rejections
// ... basically, we can't actually reject a promise unless someone
// has it handled -- which is only the case when we're returned from here
if (err != null) {
throw err;
}
});
}
console.log(chalk.cyan('Starting the flow server ...'));
const flowConfigPath = path.join(cwd, '.flowconfig');
let delegate;
this.flowStarting = new Promise(resolve => {
delegate = resolve;
});
return getFlowVersion(true)
.then(globalVersion => {
if (globalVersion === null) return;
return getFlowVersion(false).then(ourVersion => {
if (globalVersion !== ourVersion) {
return Promise.reject('__FLOW_VERSION_MISMATCH__');
}
});
})
.then(
() => new Promise(resolve => {
fs.access(flowConfigPath, err => {
if (err) {
resolve(exec(flowBinPath, ['init'], { cwd }));
} else {
resolve();
}
});
})
)
.then(() => exec(flowBinPath, ['stop'], {
cwd,
}))
.then(() => exec(flowBinPath, ['start'], { cwd }).catch(err => {
if (
typeof err.message === 'string' &&
err.message.indexOf('There is already a server running') !== -1
) {
return true;
} else {
throw err;
}
}))
.then(() => {
this.flowStarted = true;
delegate();
this.flowStarting = null;
})
.catch(err => {
delegate(err);
this.flowStarting = null;
throw err;
});
}

apply(compiler) {
compiler.plugin('compile', () => {
this.shouldRun = false;
});

compiler.plugin('compilation', compilation => {
compilation.plugin('normal-module-loader', (loaderContext, module) => {
if (
this.shouldRun ||
module.resource.indexOf('node_modules') !== -1 ||
!/[.]js(x)?$/.test(module.resource)
) {
return;
}
const contents = loaderContext.fs.readFileSync(module.resource, 'utf8');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not slowing down the build? Seems suspicious to read every file if Webpack also does it by itself.

Copy link
Contributor

@Timer Timer May 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loaderContext.fs.readFileSync is webpack's memory file system, not the real filesystem.
We're basically doing cache[key].

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aaaah.

if (
/^\s*\/\/.*@flow/.test(contents) || /^\s*\/\*.*@flow/.test(contents)
) {
this.shouldRun = true;
}
});
});

// Run lint checks
compiler.plugin('emit', (compilation, callback) => {
if (!this.shouldRun) {
callback();
return;
}
const cwd = compiler.options.context;
const first = this.flowStarting == null && !this.flowStarted;
this.startFlow(cwd)
.then(() => {
if (first) {
console.log(
chalk.yellow(
'Flow is initializing, ' +
chalk.bold('this might take a while...')
)
);
} else {
console.log('Running flow...');
}
exec(flowBinPath, ['status', '--color=always'], { cwd })
.then(() => {
callback();
})
.catch(e => {
compilation.warnings.push(formatFlowErrors(e));
callback();
});
})
.catch(e => {
if (e === '__FLOW_VERSION_MISMATCH__') {
compilation.warnings.push(createVersionWarning(this.flowVersion));
} else {
compilation.warnings.push(
'Flow: Type checking has been disabled due to an error in Flow.'
);
}
callback();
});
});
}
}

module.exports = FlowTypecheckPlugin;
4 changes: 3 additions & 1 deletion packages/react-dev-utils/formatWebpackMessages.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ function formatWebpackMessages(json) {
return 'Error in ' + formatMessage(message);
});
var formattedWarnings = json.warnings.map(function(message) {
return 'Warning in ' + formatMessage(message);
var formattedMessage = formatMessage(message);
if (/^Flow: /.test(message)) return formattedMessage;
return 'Warning in ' + formattedMessage;
});
var result = {
errors: formattedErrors,
Expand Down
2 changes: 2 additions & 0 deletions packages/react-dev-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"clearConsole.js",
"crashOverlay.js",
"FileSizeReporter.js",
"FlowTypecheckPlugin.js",
"formatWebpackMessages.js",
"getProcessForPort.js",
"InterpolateHtmlPlugin.js",
Expand All @@ -31,6 +32,7 @@
"chalk": "1.1.3",
"escape-string-regexp": "1.0.5",
"filesize": "3.3.0",
"flow-bin": "0.45.0",
"gzip-size": "3.0.0",
"html-entities": "1.2.0",
"opn": "4.0.2",
Expand Down
3 changes: 3 additions & 0 deletions packages/react-scripts/config/webpack.config.dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const HtmlWebpackPlugin = require('html-webpack-plugin');
const CaseSensitivePathsPlugin = require('case-sensitive-paths-webpack-plugin');
const InterpolateHtmlPlugin = require('react-dev-utils/InterpolateHtmlPlugin');
const WatchMissingNodeModulesPlugin = require('react-dev-utils/WatchMissingNodeModulesPlugin');
var FlowTypecheckPlugin = require('react-dev-utils/FlowTypecheckPlugin');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the other imports uses const, so this not also? :)

const getClientEnvironment = require('./env');
const paths = require('./paths');

Expand Down Expand Up @@ -241,6 +242,8 @@ module.exports = {
// makes the discovery automatic so you don't have to restart.
// See https://github.com/facebookincubator/create-react-app/issues/186
new WatchMissingNodeModulesPlugin(paths.appNodeModules),
// Run Flow on files with the @flow header
new FlowTypecheckPlugin(),
],
// Some libraries import Node modules but don't use them in the browser.
// Tell Webpack to provide empty mocks for them so importing them works.
Expand Down
3 changes: 3 additions & 0 deletions packages/react-scripts/config/webpack.config.prod.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const HtmlWebpackPlugin = require('html-webpack-plugin');
const ExtractTextPlugin = require('extract-text-webpack-plugin');
const ManifestPlugin = require('webpack-manifest-plugin');
const InterpolateHtmlPlugin = require('react-dev-utils/InterpolateHtmlPlugin');
var FlowTypecheckPlugin = require('react-dev-utils/FlowTypecheckPlugin');
const paths = require('./paths');
const getClientEnvironment = require('./env');

Expand Down Expand Up @@ -277,6 +278,8 @@ module.exports = {
new ManifestPlugin({
fileName: 'asset-manifest.json',
}),
// Run Flow on files with the @flow header
new FlowTypecheckPlugin(),
],
// Some libraries import Node modules but don't use them in the browser.
// Tell Webpack to provide empty mocks for them so importing them works.
Expand Down
6 changes: 6 additions & 0 deletions packages/react-scripts/scripts/utils/createWebpackCompiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,12 @@ module.exports = function createWebpackCompiler(config, onReadyCallback) {
chalk.yellow('/* eslint-disable */') +
' to ignore all warnings in a file.'
);
// Teach some Flow tricks.
console.log(
'Use ' +
chalk.yellow('// $FlowFixMe') +
' to ignore flow-related warnings on the next line.'
);
}
});

Expand Down
15 changes: 6 additions & 9 deletions packages/react-scripts/template/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -646,24 +646,21 @@ We suggest the following approach:

Here is an example of adding a [customized Bootstrap](https://medium.com/@tacomanator/customizing-create-react-app-aa9ffb88165) that follows these steps.

## Adding Flow
## Using Flow

Flow is a static type checker that helps you write code with fewer bugs. Check out this [introduction to using static types in JavaScript](https://medium.com/@preethikasireddy/why-use-static-types-in-javascript-part-1-8382da1e0adb) if you are new to this concept.

Recent versions of [Flow](http://flowtype.org/) work with Create React App projects out of the box.
Flow typing is supported out of the box. All you have to do is add the `/* @flow */` comment on top of files you
want to typecheck. If no `.flowconfig` is present, one will be generated for you.

To add Flow to a Create React App project, follow these steps:
Flow errors will show up alongside ESLint errors as you work on your application.

1. Run `npm install --save-dev flow-bin` (or `yarn add --dev flow-bin`).
2. Add `"flow": "flow"` to the `scripts` section of your `package.json`.
3. Run `npm run flow -- init` (or `yarn flow -- init`) to create a [`.flowconfig` file](https://flowtype.org/docs/advanced-configuration.html) in the root directory.
4. Add `// @flow` to any files you want to type check (for example, to `src/App.js`).
>Note: If your global flow installation version differs from react-scripts's flow version, you may experience slower compilation times while going back and forth between your app and your IDE (that may use your global flow). Ensure you have the same `flow-bin` version [here](https://github.com/facebookincubator/create-react-app/blob/master/packages/react-dev-utils/package.json).

Now you can run `npm run flow` (or `yarn flow`) to check the files for type errors.
You can optionally use an IDE like [Nuclide](https://nuclide.io/docs/languages/flow/) for a better integrated experience.
In the future we plan to integrate it into Create React App even more closely.

To learn more about Flow, check out [its documentation](https://flowtype.org/).
You may also want to learn how to use definitions from [flow-typed](https://github.com/flowtype/flow-typed).

## Adding Custom Environment Variables

Expand Down
20 changes: 20 additions & 0 deletions tasks/e2e-simple.sh
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ exists build/static/js/*.js
exists build/static/css/*.css
exists build/static/media/*.svg
exists build/favicon.ico
test ! -e .flowconfig

# Run tests with CI flag
CI=true npm test
Expand All @@ -239,6 +240,16 @@ npm start -- --smoke-test
# Test environment handling
verify_env_url

# Test optional flow enabling
cp src/App.js src/App.backup.js
echo "/* @flow */" > src/App.js
cat src/App.backup.js >> src/App.js
npm start -- --smoke-test
test -e .flowconfig
rm src/App.js
cp src/App.backup.js src/App.js
rm src/App.backup.js

# ******************************************************************************
# Finally, let's check that everything still works after ejecting.
# ******************************************************************************
Expand Down Expand Up @@ -275,5 +286,14 @@ npm start -- --smoke-test
# Test environment handling
verify_env_url

# Test optional flow enabling
cp src/App.js src/App.backup.js
cp .gitignore .gitignore.backup
echo "/* @flow */" > src/App.js
cat src/App.backup.js >> src/App.js
npm start -- --smoke-test
cp src/App.backup.js src/App.js
rm src/App.backup.js

# Cleanup
cleanup