Code style, testing etc.

General approach

In general, we prefer

• functional-style Javascript, using ES6 syntax (fat-arrow functions, destructuring, etc)
• shorter functions with good names, and easier testability, compared to very long and complex functions
• pure React components (functions) when possible (ie. no life-cycle events or state necessary - we're also experimenting with things like recompose, see the withVisibility HOC)
• shorter and more focused React components, rather than one large and complex one

Formatting

We use prettier for all JS files, with the only special setting being --single-quotes. There are plugins for all major editors.

Running npm start eslint.fix from the root applies prettier formatting to all files, and runs ESlint on the whole project (we have excluded all ESLint errors related to formatting, other than checking that Prettier has been applied to the project).

Note that prettier is a young project, and new versions still occasionally changes formatting. If you use an editor plugin, make sure that it either uses the prettier version in the repository, or that your global installation has the same version number. We will occasionally update the prettier version, and check it in with a diff of all files reformatted.

Testing

We are very interested in maintaining code quality and stability in a fast-changing very complex project. We use a number of different tools, but hope to expand our efforts substantially, and welcome contributions and ideas.

ESLint

We use ESLint to check for common mistakes or bad practices, such as declaring variables that are not used, etc. Many tedious errors are avoided through the use of prettier, and we have had to turn off many errors that we don't agree with. This is an ongoing process. npm start eslint.fix reformats the entire code base, and runs ESLint. This is recommended before submitting pull requests. npm start eslint only runs ESLint (without reformatting). We recommend using editor plugins that show you ESLint errors inline, while editing.

Flow

Flow is a static type-checker for Javascript. It is especially important to keep internal APIs and API users in sync, but has value almost everywhere. Due to the nature of the Javascript ecosystem, it is very difficult to aim for 100% coverage, but we are gradually expanding the use of Flow, and all new files should have Flow turned on as a general principle.

Flow only checks files with // @flow as the first line. The only required annotations are on exports, all other functions can be interpreted, however it can add to documentation and improve errors to add annotations to all functions.

We have a set of shared type definitions in ./frog-utils/src/types.js. It is expected that all operators and activities will import and use the appropriate types (ActivityPackageT for example), as well as all functions operating on any of the defined data types (activityData and socialStructure).

Again, there are good editor plugins for most editors. You can also run npm start flow from the root directory.

Jest

Jest is one of many JS test frameworks, and the one we have chosen. Our approach is to put test files in a __tests__ directory at the same level as the files you want to test (so if you have a directory called imports/api/users.js, then tests should be in imports/api/__tests__/users.js. The name of the test file does not have to match the name of the file to be tested -- if you have an index.js file which exports a few function, you could write one test file for each function, or even multiple test files to test multiple aspects of a single function, if that makes the code more manageable.

We have just recently begun adding jest tests, so coverage is quite low, but new code is expected to have tests where appropriate. Pure functions that manipulate data are fairly easy to test, but tests should cover multiple cases, including whether the function deals properly with wrong/mismatched data, empty data, etc. When discovering/fixing bugs, when possible, adding tests covering these bugs (which the pull-request turns green) is a great way of avoiding future regressions. Tests are also good documentation. Test files should themselves be Flow typechecked, and it's a good practice to import and use type definitions from frog-utils whenever a function is manipulating one of the spec'ed data formats, to make sure both the test input, and the expected test output, have the correct type.

To make it easier to write tests, prefer pure functions that take data in, and return data, and when wrapping these in MobX stores, or Meteor createCollection, etc., do this as a separate function, so that the unwrapped (pure) function can be exported and tested directly, without having to mock databases etc. (This is not always the case in the existing code base). This includes React components.

Currently testing files with import from Meteor packages and collections is very difficult (we need to create mocks). In many cases, you can extract the pure functionality to be tested into a separate file, which is then imported into the test file.

For React components, we can use snapshot testing (although verify that the snapshot is correct when checking it in the first time).