Skip to content

Commit

Permalink
ESLinting (#3850)
Browse files Browse the repository at this point in the history
* Removing @typescript-eslint/eslint-recommended

Since plugin:@typescript-eslint/recommended is already extending it.

* Marking eslint configs as root

* Removing unused tslint.json configurations

* Using eslint in realm-intergration-tests

* Reordering plugins in the config

* Adding lerna: scripts to root package

* Publishing realm-app-importer on the default repository

* Adding a lint script to electron and node

* Fixed trivial false-negatives

* Ignoring the example packages

* Creating a workflow to lint on PRs

* Hoisting eslint to the root

* Switched around name and run in workflow

* Simplifying eslint configs by relying on cascading

* A more default prettier config

* Minor adjustments to the eslint configs

* Updated header to match the copyright date as a pattern

* Simplified eslintconfig in types

* Upgraded to node v14 & npm v7 in workflows

* Adding npm >= 7 to engines

* Removed linting in the realm-web workflow

* Minor change to the eslint configs

* Don't use npm v7 on windows

* Renamed job

* Fixing eslint config errors

* Adding a guide and vscode configuration files

* Removed unwanted dependecies on tslint

* Swapped if: and run: in workflow

* Removed linting from test.sh and Jenkinsfile

* Updated package-locks
  • Loading branch information
kraenhansen authored Jul 16, 2021
1 parent 36944e8 commit 2e5819b
Show file tree
Hide file tree
Showing 48 changed files with 1,969 additions and 1,999 deletions.
2 changes: 2 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,5 @@ vendor/
# The sub-packages have their own linting
/integration-tests/
/packages/
/examples/
/tests/ReactTestApp/
30 changes: 8 additions & 22 deletions .eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,29 +1,15 @@
{
"root": true,
"extends": [
"eslint:recommended",
"plugin:prettier/recommended",
"./header.eslintrc"
],
"env": {
"es2017": true,
"node": true
"es2017": true
},
"extends": "eslint:recommended",
"rules": {
"comma-dangle": 0,
"no-empty": 0,
"no-console": 0,
"no-unused-vars": [
"warn",
{
"varsIgnorePattern": "^_",
"args": "none"
}
],
"strict": [
2,
"global"
],
"quotes": [
1,
"double"
]
"parserOptions": {
"ecmaVersion": 2018
},
"settings": {
"react": {
Expand Down
16 changes: 13 additions & 3 deletions .github/workflows/integration-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ jobs:
with:
node-version: ${{ matrix.node }}
registry-url: https://registry.npmjs.org/
- name: Install npm v7
# TODO: Remove the following line once we've found a workaround for https://github.com/dotnet/msbuild/issues/5726
if: ${{ matrix.os != 'windows-latest' }}
run: npm install -g npm@7
# The following is a noop on non-windows runners
- name: Install Microsoft Visual C++ cmd tools
uses: ilammy/msvc-dev-cmd@v1
Expand Down Expand Up @@ -71,8 +75,10 @@ jobs:
submodules: 'recursive'
- uses: actions/setup-node@v1
with:
node-version: 12
node-version: 14
registry-url: https://registry.npmjs.org/
- name: Install npm v7
run: npm install -g npm@7
# Install the root package (--ignore-scripts to avoid downloading or building the native module)
- run: npm ci --ignore-scripts
# Bootstrap lerna sub-packages (builds the packages and Realm JS native module)
Expand Down Expand Up @@ -103,8 +109,10 @@ jobs:
submodules: 'recursive'
- uses: actions/setup-node@v1
with:
node-version: 12
node-version: 14
registry-url: https://registry.npmjs.org/
- name: Install npm v7
run: npm install -g npm@7
# Setup tools
- name: Install tools
run: brew install watchman
Expand Down Expand Up @@ -181,8 +189,10 @@ jobs:
submodules: 'recursive'
- uses: actions/setup-node@v1
with:
node-version: 12
node-version: 14
registry-url: https://registry.npmjs.org/
- name: Install npm v7
run: npm install -g npm@7
# Setup tools
- name: Install tools
run: brew install watchman
Expand Down
28 changes: 28 additions & 0 deletions .github/workflows/pr-linting.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
name: Linting (Pull Request)

on:
pull_request:
paths:
# No need to run when updating documentation
- '!**.md'

jobs:
lint:
name: Lint
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
with:
node-version: 14
registry-url: https://registry.npmjs.org/
- name: Install npm v7
run: npm install -g npm@7
# Install the root package (--ignore-scripts to avoid downloading or building the native module)
- name: Install root package dependencies
run: npm ci --ignore-scripts
- name: Install subpackages dependencies
run: npm run lerna:bootstrap
- name: Run linting of subpackages
run: npm run lerna:lint -- --no-bail --concurrency 1 --stream --no-prefix
12 changes: 6 additions & 6 deletions .github/workflows/pr-realm-web.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,23 @@ on:
- '!**.md'

jobs:
test-build-and-publish:
name: Test, lint and build
job:
name: Build & test
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
with:
node-version: 12
node-version: 14
registry-url: https://registry.npmjs.org/
- name: Install npm v7
run: npm install -g npm@7
# Install the root package (--ignore-scripts to avoid downloading or building the native module)
- run: npm ci --ignore-scripts
# Bootstrap lerna sub-packages
- run: npx lerna bootstrap --scope realm-web-integration-tests --include-dependencies
# Lint, build and test the package
- run: npm run lint
working-directory: packages/realm-web
# Build and test the package
- run: npm run build
working-directory: packages/realm-web
- name: Run unit tests
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/publish-realm-web.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-node@v1
with:
node-version: 12
node-version: 14
registry-url: https://registry.npmjs.org/
- name: Install npm v7
run: npm install -g npm@7
# Install the root package
- run: npm ci
# Bootstrap lerna sub-packages
Expand Down
5 changes: 1 addition & 4 deletions .prettierrc.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
module.exports = {
jsxBracketSameLine: true,
trailingComma: 'all',
tabWidth: 4,
arrowParens: 'avoid',
// printWidth: 120,
printWidth: 120,
};
5 changes: 5 additions & 0 deletions .vscode/extensions.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"recommendations": [
"dbaeumer.vscode-eslint"
]
}
7 changes: 7 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"eslint.workingDirectories": [
{
"mode": "auto"
}
]
}
12 changes: 0 additions & 12 deletions Jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -84,18 +84,6 @@ if (packagesExclusivelyChanged) {

stage('pretest') {
parallelExecutors = [:]
parallelExecutors["eslint"] = testLinux("eslint-ci Release ${nodeTestVersion}", { // "Release" is not used
step([
$class: 'CheckStylePublisher',
canComputeNew: false,
canRunOnFailed: true,
defaultEncoding: '',
healthy: '',
pattern: 'eslint.xml',
unHealthy: '',
maxWarnings: 0,
ignoreFailures: false])
})
parallelExecutors["jsdoc"] = testLinux("jsdoc Release ${nodeTestVersion}", { // "Release is not used
publishHTML([
allowMissing: false,
Expand Down
44 changes: 44 additions & 0 deletions contrib/linting.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Keeping our code style aligned with linting

We're using linters for a couple of reasons:
- We've experienced a lot of reviews to focus on the stying of code rather than its design and architecture. In particular, this has the potential to be a real turn-off for a newcomer.
- Writing code in a shared style makes way easier to read across files.
- As with any static analysis of code, linting has the potential to find bugs before our users do.

## Setup your editor to format on save

To help yourself keep your code passing linters at all times, it's highly recommended to setup your editor to help you.

### Visual Studio Code

- Install and enable the ["eslint" extension for VS Code](https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint).
- Make sure you've enabled "format on save" on your workspace.

Both the eslint plugin and formatting on save can be enabled via the UI or you can <kbd>Cmd</kbd> + <kbd>P</kbd>, type *"> Preferences Open Workspace Settings (JSON)"* and paste in the following in the `settings` object:

```json
"eslint.format.enable": true,
"editor.formatOnSave": true
```

## Running the linters off the CLI

Linters are run for every pull-request, via [the `pr-linting` workflow](../.github/workflows/pr-linting.yml) on GitHub Actions.

Every package of this mono-repository should have a `lint` script - to lint, simply change your working directory and run the script:

```
npm run lint
```

To run linting on every package of the project, use the `lerna:lint` script in the root package:

```
npm run lerna:lint
```

The linter can automatically fix some issues. To run a fix across every sub-package, you can use the `lerna:lint:fix` script in the root package:

```
npm run lerna:lint:fix
```
3 changes: 1 addition & 2 deletions examples/ReactExample/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
"moment": "2.19.1"
},
"devDependencies": {
"eslint": "^4.18.2",
"fs-extra": "^4.0.3",
"graceful-fs": "^4.2.6",
"install-local": "^1.0.0",
Expand All @@ -29,4 +28,4 @@
"localDependencies": {
"realm": "../.."
}
}
}
48 changes: 48 additions & 0 deletions header.eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
////////////////////////////////////////////////////////////////////////////
//
// Copyright 2021 Realm Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
////////////////////////////////////////////////////////////////////////////

/* eslint-env node */

const currentYear = new Date().getFullYear();

const header = [
"//////////////////////////////////////////////////////////////////////////",
"",
{ pattern: " Copyright \\d{4} Realm Inc.", template: ` Copyright ${currentYear} Realm Inc.` },
"",
' Licensed under the Apache License, Version 2.0 (the "License");',
" you may not use this file except in compliance with the License.",
" You may obtain a copy of the License at",
"",
" http://www.apache.org/licenses/LICENSE-2.0",
"",
" Unless required by applicable law or agreed to in writing, software",
' distributed under the License is distributed on an "AS IS" BASIS,',
" WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.",
" See the License for the specific language governing permissions and",
" limitations under the License.",
"",
"//////////////////////////////////////////////////////////////////////////",
];

module.exports = {
plugins: ["header"],
rules: {
"header/header": ["error", "line", header],
},
};
17 changes: 0 additions & 17 deletions header.js

This file was deleted.

9 changes: 4 additions & 5 deletions integration-tests/environments/electron/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
{
"extends": "../../../.eslintrc.json",
"parserOptions": {
"ecmaVersion": 2018
}
}
"env": {
"node": true
}
}
3 changes: 2 additions & 1 deletion integration-tests/environments/electron/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"private": true,
"main": "app/main.js",
"scripts": {
"lint": "eslint .",
"package": "electron-builder --dir",
"runner:main": "node runner.js main",
"runner:renderer": "node runner.js renderer",
Expand Down Expand Up @@ -53,4 +54,4 @@
"!node_modules/realm/vendor${/*}"
]
}
}
}
6 changes: 3 additions & 3 deletions integration-tests/environments/node/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"extends": "../../../.eslintrc.json",
"env": {
"mocha": true
"mocha": true,
"node": true
}
}
}
5 changes: 3 additions & 2 deletions integration-tests/environments/node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
"description": "Realm JS tests running in a Node.js environment",
"private": true,
"scripts": {
"test": "mocha-remote -- concurrently npm:app-importer npm:start",
"start": "node index.js",
"test": "mocha-remote -- concurrently npm:app-importer npm:start",
"lint": "eslint .",
"app-importer": "realm-app-importer serve ../../realm-apps"
},
"dependencies": {
Expand All @@ -27,4 +28,4 @@
"concurrently": "^6.0.2",
"realm-app-importer": "*"
}
}
}
4 changes: 2 additions & 2 deletions integration-tests/environments/react-native/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"extends": [
"@react-native-community",
"../../../.eslintrc.json"
"@react-native-community",
"../../../.eslintrc"
]
}
Loading

0 comments on commit 2e5819b

Please sign in to comment.