Skip to content

Commit 87d27ef

Browse files
authored
Migrate from prettier to eslint to match firebase/firebase-tools styling (#1194)
Let's bite the bullet - match styling of firebase/firebase-tools and start using eslint over prettier. This change effectively touches almost all files in this repo, especially since eslint prefers double-quotes over single-quotes. I've made couple of other changes to make eslint errors go away. Things like: 1) Eliminate dangling promises 2) Remove empty function definition (prefer `()=>undefined` over `()=>{}`) 3) Remove unused arguments from function definitions In theory, these are just linter changes without changes to the runtime behavior. v4 launches will need good tests and bugbash anyway, so this feels like a great time to commit to this migration.
1 parent 0e1f688 commit 87d27ef

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+4377
-2119
lines changed

.eslintignore

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
coverage
2+
dev
3+
lib
4+
node_modules
5+
docgen
6+
v1
7+
v2
8+
logger
9+
dist
10+
spec/fixtures
11+
scripts/**/*.js

.eslintrc.js

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
module.exports = {
2+
env: {
3+
es6: true,
4+
node: true,
5+
},
6+
extends: [
7+
"eslint:recommended",
8+
"plugin:@typescript-eslint/recommended",
9+
"plugin:@typescript-eslint/recommended-requiring-type-checking",
10+
"plugin:jsdoc/recommended",
11+
"google",
12+
"prettier",
13+
],
14+
rules: {
15+
"jsdoc/newline-after-description": "off",
16+
"jsdoc/require-jsdoc": ["warn", { publicOnly: true }],
17+
"no-restricted-globals": ["error", "name", "length"],
18+
"prefer-arrow-callback": "error",
19+
"prettier/prettier": "error",
20+
"require-atomic-updates": "off", // This rule is so noisy and isn't useful: https://github.com/eslint/eslint/issues/11899
21+
"require-jsdoc": "off", // This rule is deprecated and superseded by jsdoc/require-jsdoc.
22+
"valid-jsdoc": "off", // This is deprecated but included in recommended configs.
23+
24+
"no-prototype-builtins": "warn",
25+
"no-useless-escape": "warn",
26+
"prefer-promise-reject-errors": "warn",
27+
},
28+
overrides: [
29+
{
30+
files: ["*.ts"],
31+
rules: {
32+
"jsdoc/require-param-type": "off",
33+
"jsdoc/require-returns-type": "off",
34+
35+
// Google style guide allows us to omit trivial parameters and returns
36+
"jsdoc/require-param": "off",
37+
"jsdoc/require-returns": "off",
38+
39+
"@typescript-eslint/no-invalid-this": "error",
40+
"@typescript-eslint/no-unused-vars": "error", // Unused vars should not exist.
41+
"@typescript-eslint/no-misused-promises": "warn", // rule does not work with async handlers for express.
42+
"no-invalid-this": "off", // Turned off in favor of @typescript-eslint/no-invalid-this.
43+
"no-unused-vars": "off", // Off in favor of @typescript-eslint/no-unused-vars.
44+
eqeqeq: ["error", "always", { null: "ignore" }],
45+
camelcase: ["error", { properties: "never" }], // snake_case allowed in properties iif to satisfy an external contract / style
46+
47+
// Ideally, all these warning should be error - let's fix them in the future.
48+
"@typescript-eslint/no-unsafe-argument": "warn",
49+
"@typescript-eslint/no-unsafe-assignment": "warn",
50+
"@typescript-eslint/no-unsafe-call": "warn",
51+
"@typescript-eslint/no-unsafe-member-access": "warn",
52+
"@typescript-eslint/no-unsafe-return": "warn",
53+
"@typescript-eslint/restrict-template-expressions": "warn",
54+
},
55+
},
56+
{
57+
files: ["*.spec.*"],
58+
env: {
59+
mocha: true,
60+
},
61+
rules: {},
62+
},
63+
],
64+
globals: {},
65+
parserOptions: {
66+
project: "tsconfig.json",
67+
},
68+
plugins: ["prettier", "@typescript-eslint", "jsdoc"],
69+
settings: {
70+
jsdoc: {
71+
tagNamePreference: {
72+
returns: "return",
73+
},
74+
},
75+
},
76+
parser: "@typescript-eslint/parser",
77+
};

.github/ISSUE_TEMPLATE/---report-a-bug.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
---
2-
name: '⚠️ Report a Bug'
2+
name: "⚠️ Report a Bug"
33
about: Think you found a bug in the firebase-functions SDK? Report it here. Please do not use this form if your function is deployed successfully but not working as you expected.
4-
title: ''
5-
labels: ''
6-
assignees: ''
4+
title: ""
5+
labels: ""
6+
assignees: ""
77
---
88

99
<!-- DO NOT DELETE

.github/workflows/postmerge.yaml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ name: Post-merge tests
1616
on:
1717
workflow_dispatch:
1818
workflow_run:
19-
workflows: ['CI Tests']
19+
workflows: ["CI Tests"]
2020
types: [completed]
2121
branches: [master]
2222

@@ -38,16 +38,16 @@ jobs:
3838

3939
- uses: google-github-actions/auth@v0
4040
with:
41-
credentials_json: '${{ secrets.CF3_INTEGRATION_TEST_GOOGLE_CREDENTIALS }}'
41+
credentials_json: "${{ secrets.CF3_INTEGRATION_TEST_GOOGLE_CREDENTIALS }}"
4242
create_credentials_file: true
4343

44-
- name: 'Set up Cloud SDK'
44+
- name: "Set up Cloud SDK"
4545
uses: google-github-actions/setup-gcloud@v0
4646

47-
- name: 'Setup Firebase CLI'
47+
- name: "Setup Firebase CLI"
4848
run: npm i -g firebase-tools
4949

50-
- name: 'Run integration test'
50+
- name: "Run integration test"
5151
run: npm run test:postmerge
5252

5353
- name: Print debug logs

.github/workflows/test.yaml

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,23 @@ env:
88
CI: true
99

1010
jobs:
11+
lint:
12+
runs-on: ubuntu-latest
13+
if: github.event_name == 'pull_request'
14+
strategy:
15+
matrix:
16+
node-version:
17+
- "16"
18+
steps:
19+
- uses: actions/checkout@v3
20+
with:
21+
fetch-depth: 0
22+
- uses: actions/setup-node@v3
23+
with:
24+
node-version: ${{ matrix.node-version }}
25+
cache: npm
26+
- run: npm ci
27+
- run: npm run lint
1128
unit:
1229
runs-on: ubuntu-latest
1330
strategy:
@@ -20,18 +37,15 @@ jobs:
2037
- uses: actions/setup-node@v1
2138
with:
2239
node-version: ${{ matrix.node-version }}
23-
2440
- name: Cache npm
2541
uses: actions/cache@v1
2642
with:
2743
path: ~/.npm
2844
key: ${{ runner.os }}-node-${{ matrix.node-version }}-${{ hashFiles('**/package-lock.json') }}
29-
3045
- run: npm ci
31-
- run: npm run lint
32-
- run: npm run format
3346
- run: npm run test
3447
integration:
48+
needs: "unit"
3549
runs-on: ubuntu-latest
3650
strategy:
3751
matrix:
@@ -43,12 +57,10 @@ jobs:
4357
- uses: actions/setup-node@v1
4458
with:
4559
node-version: ${{ matrix.node-version }}
46-
4760
- name: Cache npm
4861
uses: actions/cache@v1
4962
with:
5063
path: ~/.npm
5164
key: ${{ runner.os }}-node-${{ matrix.node-version }}-${{ hashFiles('**/package-lock.json') }}
52-
53-
- run: npm install
65+
- run: npm ci
5466
- run: npm run test:bin

.mocharc.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@ extension:
44
package: ./package.json
55
reporter: spec
66
require:
7-
- 'ts-node/register'
8-
- 'source-map-support/register'
7+
- "ts-node/register"
8+
- "source-map-support/register"

.prettierignore

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1-
lib
2-
package.json
3-
spec/fixtures/credential/unparsable.key.json
1+
/node_modules
2+
/lib/**/*
3+
/CONTRIBUTING.md
4+
/docgen

.prettierrc

Lines changed: 0 additions & 5 deletions
This file was deleted.

.prettierrc.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
module.exports = {
2+
printWidth: 100,
3+
};

README.md

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,13 @@ _Please avoid double posting across multiple channels!_
2323

2424
```js
2525
// functions/index.js
26-
const functions = require('firebase-functions');
27-
const notifyUsers = require('./notify-users');
28-
29-
exports.newPost = functions.database
30-
.ref('/posts/{postId}')
31-
.onCreate((snapshot, context) => {
32-
functions.logger.info('Received new post with ID:', context.params.postId);
33-
return notifyUsers(snapshot.val());
34-
});
26+
const functions = require("firebase-functions");
27+
const notifyUsers = require("./notify-users");
28+
29+
exports.newPost = functions.database.ref("/posts/{postId}").onCreate((snapshot, context) => {
30+
functions.logger.info("Received new post with ID:", context.params.postId);
31+
return notifyUsers(snapshot.val());
32+
});
3533
```
3634

3735
## Contributing

0 commit comments

Comments
 (0)