Install flow tool

[veronicabenedict]

Installed the flow static analysis tool by running npm install --save-dev @babel/core @babel/cli @babel/preset-flow babel-plugin-syntax-hermes-parser in the terminal and adding a script and devDependencies to the ./package.json file.


By installing flow, .babelrc and .flowconfig files were added to the root directory of the repo. Additionally, the ./node_modules/.bin/babel src/ -d lib/ was run to take all JavaScript files in the src/ directory and transpile them with Babel, and output those files into the lib/ directory.
Once installed, the command npm run flow init and then npm run flow which produced the following output in the terminal.

[coveralls]

Pull Request Test Coverage Report for Build 11504166815

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage decreased (-0.007%) to 82.683%

---

Files with Coverage Reduction	New Missed Lines	%
src/meta/errors.js	1	76.74%

Totals
Change from base Build 11487961485:	-0.007%
Covered Lines:	22379
Relevant Lines:	25641

---

💛 - Coveralls

[veronicabenedict]

Removed /lib/ directory as transpiled file as adding them to the main branch can lead to merge conflicts and should be created during CI not stored in the repo. Added /lib/ to the .gitignore file.

[etong11]

Integration

Adding dependencies to GitHub

We followed the set-up documentation that Flow provides for installation, adding the dev dependency packages that were installed with the npm install commands to install/package.json since the package.json in the root directory is in the .gitignore. We also discovered that the ./nodebb setup command installs packages located in this file.

Using the documentation as guidance, we added packages @babel/core, @babel/cli, @babel/preset-flow, and babel-plugin-syntax-hermes-parser. We also created a .babelrc file in our root directory that would be tracked by git and added the given configuration to it:

{
  "presets": ["@babel/preset-flow"],
  "plugins": ["babel-plugin-syntax-hermes-parser"],
}

Flow also needed to be added to the scripts in install/package.json. To integrate Flow into the development process with an IDE, we enabled the “Flow Language Support” extension in VSCode to allow type annotations in Javascript files so that we can use the tool’s type annotations without it being flagged by the IDE during development; without it, these annotations are flagged since type annotations are not supported in Javascript files.

Finally, we modified a few files that we worked with in Project 2 to use Flow type annotations which serve as an example of how we would use this tool during development:

• src/user/admin.js
• src/topics/data.js
• src/topics/create.js

To do so, we added // @flow to the top of the file and fixed the issues flagged by the tool with type annotations.

GitHub Workflow

We modified the existing workflow file that runs currently runs ESLint. After this file copies the package.json in install/package.json to the root directory and installs the packages with npm, we added a step to initialize flow with npm run flow init and run flow with npm run flow.

Changes to Project Configuration / Addressing Build Failures

npm run test and ./nodebb setup

After adding the type annotations to files and running npm run test and ./nodebb setup, the tests initially failed due to a syntax error from the type annotation (use of :). To fix this, we had to add @babel/register to the dev dependencies and modify the test script in config.json to "test": "nyc --reporter=html --reporter=text-summary mocha --require @babel/register". This modification allows the tests to run without failing on the Flow type annotations although it still logs it as a syntax error in the console.

The setup had a similar issue with the type annotations. We fixed this by adding require('@babel/register') to loader.js and src/cli/setup.js which we believe are run during the ./nodebb setup command. This fixed the syntax error for the type annotations arising in the “Setup for Redis” GitHub action, allowing for following actions to be run. However, this means that in order for this dependency to be recognized during setup, when developing locally, developers should copy the install/package.json to their root directory and run npm install before running the setup command.

Locally, a majority of the tests are passing; however, in GitHub, certain tests are failing likely due to faulty tests or time-out issues since we did not modify any of the files that the tests are failing for. Here is a screenshot of the tests passing locally with Flow:

npm run lint

Due to the flow annotations, the ESLint linter failed as well. Flow had existing documentation that we tried using to address these failures. We added new dev dependencies hermes-eslint and eslint-plugin-ft-flow which are plugins and parsers compatible with Flow which should disable some rules from the default ESLint configuration that fails with flow type annotations. We also modified the .eslintrc file to change the existing ESLint configuration and use these new dependencies:

{
    "root": true,
    "parser": "hermes-eslint",
    "plugins": [
        "ft-flow"
    ],
    "extends": [
        "plugin:ft-flow/recommended",
        "nodebb"
    ],
    "env": {
        "node": true
    }
}

However, these changes gave us new linter errors that were not solely related to files that we added the flow annotation to such as error Do not access Object.prototype method 'hasOwnProperty' from target object no-prototype-builtins. As we made more modifications to get the linter to work, we were getting different failures with the linter that weren't solely related to the Flow tool. For example, there appears to be a problem with using hermes-eslint in NodeBB; changing the parser to @babel/eslint-parser runs the linter and outputs linter errors, but unmodified files that passed the original linter configuration are now flagged. In addition, this is not the parser recommended for Flow integration.

Although we followed the setup recommended in the Flow documentation, we infer that there is another ESLint configuration associated with nodebb that conflicts with the Flow ESLint configuration and is incompatible with hermes-eslint.