Enable linting and fix lint issues for app, util and logger (#1845)

* add eslint rules

* fix firebase/app lint issues

* fix firebas/util lint issues

* add eslint deps to util

* fix firebase/logger lint issues

* [AUTOMATED]: Prettier Code Styling

* remove intermediate merge messages

* add missing code during merge

* rename parameter

Author: Feiyang1

config/.eslintrc.json

@@ -20,7 +20,8 @@
                 "**/test/**/*.ts"
             ],
             "rules": {
-                "no-unused-expressions": "off"
+                "no-unused-expressions": "off",
+                "@typescript-eslint/no-explicit-any": "off"
             }
         }
     ],

packages/app/index.rn.ts

@@ -24,6 +24,7 @@ import { createFirebaseNamespace } from './src/firebaseNamespace';
  * some of our tests because of duplicate symbols, we are using require syntax
  * here
  */
+// eslint-disable-next-line
 const { AsyncStorage } = require('react-native');
 
 const _firebase = createFirebaseNamespace() as _FirebaseNamespace;

packages/app/index.ts

@@ -29,6 +29,7 @@ if (isBrowser() && 'firebase' in self) {
     Firebase library is only loaded once.
   `);
 
+  // eslint-disable-next-line
   const sdkVersion = ((self as any).firebase as FirebaseNamespace).SDK_VERSION;
   if (sdkVersion && sdkVersion.indexOf('LITE') >= 0) {
     logger.warn(`

packages/app/package.json

@@ -18,7 +18,7 @@
     "lint:fix": "eslint --fix -c .eslintrc.json '**/*.ts'",
     "build": "rollup -c",
     "dev": "rollup -c -w",
-    "test": "run-p test:browser test:node",
+    "test": "run-p lint test:browser test:node",
     "test:browser": "karma start --single-run",
     "test:browser:debug": "karma start --browsers Chrome --auto-watch",
     "test:node": "TS_NODE_CACHE=NO TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' nyc --reporter lcovonly -- mocha test/**/*.test.* --opts ../../config/mocha.node.opts",

packages/app/src/firebaseApp.ts

@@ -38,7 +38,7 @@ interface ServicesCache {
 
 // An array to capture listeners before the true auth functions
 // exist
-let tokenListeners: any[] = [];
+let tokenListeners: Array<(token: string | null) => void> = [];
 
 /**
  * Global context object for a collection of services using
@@ -175,6 +175,7 @@ export class FirebaseAppImpl implements FirebaseApp {
    * Callback function used to extend an App instance at the time
    * of service instance creation.
    */
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
   private extendApp(props: { [name: string]: any }): void {
     // Copy the object onto the FirebaseAppImpl prototype
     deepExtend(this, props);

packages/app/src/firebaseNamespace.ts

@@ -32,11 +32,11 @@ export function createFirebaseNamespace(): FirebaseNamespace {
   const namespace = createFirebaseNamespaceCore(FirebaseAppImpl);
   (namespace as _FirebaseNamespace).INTERNAL = {
     ...(namespace as _FirebaseNamespace).INTERNAL,
-    createFirebaseNamespace: createFirebaseNamespace,
-    extendNamespace: extendNamespace,
-    createSubscribe: createSubscribe,
-    ErrorFactory: ErrorFactory,
-    deepExtend: deepExtend
+    createFirebaseNamespace,
+    extendNamespace,
+    createSubscribe,
+    ErrorFactory,
+    deepExtend
   };
 
   /**

packages/app/src/firebaseNamespaceCore.ts

@@ -29,17 +29,13 @@ import {
   FirebaseServiceNamespace,
   AppHook
 } from '@firebase/app-types/private';
-import { deepExtend, patchProperty } from '@firebase/util';
+import { deepExtend, contains } from '@firebase/util';
 import { FirebaseAppImpl } from './firebaseApp';
 import { ERROR_FACTORY, AppError } from './errors';
 import { FirebaseAppLiteImpl } from './lite/firebaseAppLite';
 import { DEFAULT_ENTRY_NAME } from './constants';
 import { version } from '../../firebase/package.json';
 
-function contains(obj: object, key: string) {
-  return Object.prototype.hasOwnProperty.call(obj, key);
-}
-
 /**
  * Because auth can't share code with other components, we attach the utility functions
  * in an internal namespace to share code.
@@ -60,9 +56,11 @@ export function createFirebaseNamespaceCore(
     // as the firebase namespace.
     // @ts-ignore
     __esModule: true,
-    initializeApp: initializeApp,
-    app: app as any,
-    apps: null as any,
+    initializeApp,
+    // @ts-ignore
+    app,
+    // @ts-ignore
+    apps: null,
     SDK_VERSION: version,
     INTERNAL: {
       registerService,
@@ -82,7 +80,7 @@ export function createFirebaseNamespaceCore(
   //
   //   import * as firebase from 'firebase';
   //   which becomes: var firebase = require('firebase');
-  patchProperty(namespace, 'default', namespace);
+  namespace['default'] = namespace;
 
   // firebase.apps is a read-only getter.
   Object.defineProperty(namespace, 'apps', {
@@ -105,12 +103,12 @@ export function createFirebaseNamespaceCore(
   function app(name?: string): FirebaseApp {
     name = name || DEFAULT_ENTRY_NAME;
     if (!contains(apps, name)) {
-      throw ERROR_FACTORY.create(AppError.NO_APP, { name: name });
+      throw ERROR_FACTORY.create(AppError.NO_APP, { name });
     }
     return apps[name];
   }
 
-  patchProperty(app, 'App', firebaseAppImpl);
+  app['App'] = firebaseAppImpl;
   /**
    * Create a new App instance (name must be unique).
    */
@@ -119,7 +117,10 @@ export function createFirebaseNamespaceCore(
     config?: FirebaseAppConfig
   ): FirebaseApp;
   function initializeApp(options: FirebaseOptions, name?: string): FirebaseApp;
-  function initializeApp(options: FirebaseOptions, rawConfig = {}) {
+  function initializeApp(
+    options: FirebaseOptions,
+    rawConfig = {}
+  ): FirebaseApp {
     if (typeof rawConfig !== 'object' || rawConfig === null) {
       const name = rawConfig;
       rawConfig = { name };
@@ -138,7 +139,7 @@ export function createFirebaseNamespaceCore(
     }
 
     if (contains(apps, name)) {
-      throw ERROR_FACTORY.create(AppError.DUPLICATE_APP, { name: name });
+      throw ERROR_FACTORY.create(AppError.DUPLICATE_APP, { name });
     }
 
     const app = new firebaseAppImpl(
@@ -177,7 +178,7 @@ export function createFirebaseNamespaceCore(
   ): FirebaseServiceNamespace<FirebaseService> {
     // Cannot re-register a service that already exists
     if (factories[name]) {
-      throw ERROR_FACTORY.create(AppError.DUPLICATE_SERVICE, { name: name });
+      throw ERROR_FACTORY.create(AppError.DUPLICATE_SERVICE, { name });
     }
 
     // Capture the service factory for later service instantiation
@@ -194,12 +195,12 @@ export function createFirebaseNamespaceCore(
     }
 
     // The Service namespace is an accessor function ...
-    function serviceNamespace(appArg: FirebaseApp = app()) {
+    function serviceNamespace(appArg: FirebaseApp = app()): FirebaseService {
       if (typeof appArg[name] !== 'function') {
         // Invalid argument.
         // This happens in the following case: firebase.storage('gs:/')
         throw ERROR_FACTORY.create(AppError.INVALID_APP_ARGUMENT, {
-          name: name
+          name
         });
       }
 
@@ -224,7 +225,7 @@ export function createFirebaseNamespaceCore(
     return serviceNamespace;
   }
 
-  function callAppHooks(app: FirebaseApp, eventName: string) {
+  function callAppHooks(app: FirebaseApp, eventName: string): void {
     for (const serviceName of Object.keys(factories)) {
       // Ignore virtual services
       const factoryName = useAsService(app, serviceName);

packages/app/src/lite/firebaseAppLite.ts

@@ -157,7 +157,7 @@ export class FirebaseAppLiteImpl implements FirebaseApp {
    * Callback function used to extend an App instance at the time
    * of service instance creation.
    */
-  private extendApp(props: { [name: string]: any }): void {
+  private extendApp(props: { [name: string]: unknown }): void {
     // Copy the object onto the FirebaseAppImpl prototype
     deepExtend(this, props);
   }

packages/app/src/lite/firebaseNamespaceLite.ts

@@ -20,7 +20,9 @@ import {
   _FirebaseApp,
   _FirebaseNamespace,
   FirebaseServiceFactory,
-  AppHook
+  AppHook,
+  FirebaseServiceNamespace,
+  FirebaseService
 } from '@firebase/app-types/private';
 import { FirebaseAppLiteImpl } from './firebaseAppLite';
 import { createFirebaseNamespaceCore } from '../firebaseNamespaceCore';
@@ -41,10 +43,10 @@ export function createFirebaseNamespaceLite(): FirebaseNamespace {
   function registerServiceForLite(
     name: string,
     createService: FirebaseServiceFactory,
-    serviceProperties?: { [prop: string]: any },
+    serviceProperties?: { [prop: string]: unknown },
     appHook?: AppHook,
     allowMultipleInstances?: boolean
-  ) {
+  ): FirebaseServiceNamespace<FirebaseService> {
     // only allow performance to register with firebase lite
     if (name !== 'performance' && name !== 'installations') {
       throw Error(`${name} cannot register with the standalone perf instance`);

packages/app/test/firebaseApp.test.ts

@@ -28,7 +28,7 @@ import { assert } from 'chai';
 executeFirebaseTests();
 executeFirebaseLiteTests();
 
-function executeFirebaseTests() {
+function executeFirebaseTests(): void {
   firebaseAppTests('Firebase App Tests', createFirebaseNamespace);
 
   describe('Firebase Service Registration', () => {
@@ -40,24 +40,24 @@ function executeFirebaseTests() {
     it('Register App Hook', done => {
       const events = ['create', 'delete'];
       let hookEvents = 0;
-      let app: FirebaseApp;
       (firebase as _FirebaseNamespace).INTERNAL.registerService(
         'test',
         (app: FirebaseApp) => {
           return new TestService(app);
         },
         undefined,
-        (event: string, app: FirebaseApp) => {
+        (event: string, _app: FirebaseApp) => {
           assert.equal(event, events[hookEvents]);
           hookEvents += 1;
           if (hookEvents === events.length) {
             done();
           }
         }
       );
-      app = firebase.initializeApp({});
+      const app = firebase.initializeApp({});
       // Ensure the hook is called synchronously
       assert.equal(hookEvents, 1);
+      // tslint:disable-next-line:no-floating-promises
       app.delete();
     });
 
@@ -140,7 +140,7 @@ function executeFirebaseTests() {
           return new TestService(app);
         },
         undefined,
-        (event: string, app: FirebaseApp) => {
+        (event: string, _app: FirebaseApp) => {
           assert.equal(event, events[hookEvents]);
           hookEvents += 1;
           if (hookEvents === events.length) {
@@ -150,6 +150,7 @@ function executeFirebaseTests() {
       );
       // Ensure the hook is called synchronously
       assert.equal(hookEvents, 1);
+      // tslint:disable-next-line:no-floating-promises
       app.delete();
     });
 
@@ -221,6 +222,7 @@ function executeFirebaseTests() {
       );
       assert.strictEqual(service, service2);
     });
+
     it(`Should pass null to the factory method if using default instance`, () => {
       // Register Multi Instance Service
       (firebase as _FirebaseNamespace).INTERNAL.registerService(
@@ -235,11 +237,8 @@ function executeFirebaseTests() {
         }
       );
       firebase.initializeApp({});
-
-      // Capture a given service ref
-      const serviceIdentifier = 'custom instance identifier';
-      const service = (firebase.app() as any).testService();
     });
+
     it(`Should extend INTERNAL per app instance`, () => {
       let counter: number = 0;
       (firebase as _FirebaseNamespace).INTERNAL.registerService(
@@ -280,7 +279,7 @@ function executeFirebaseTests() {
   });
 }
 
-function executeFirebaseLiteTests() {
+function executeFirebaseLiteTests(): void {
   firebaseAppTests('Firebase App Lite Tests', createFirebaseNamespaceLite);
 
   describe('Firebase Lite Service Registration', () => {
@@ -291,14 +290,13 @@ function executeFirebaseLiteTests() {
     });
 
     it('should allow Performance service to register', () => {
-      let app: FirebaseApp;
       (firebase as _FirebaseNamespace).INTERNAL.registerService(
         'performance',
         (app: FirebaseApp) => {
           return new TestService(app);
         }
       );
-      app = firebase.initializeApp({});
+      const app = firebase.initializeApp({});
       const perf = (app as any).performance();
       assert.isTrue(perf instanceof TestService);
     });
@@ -316,7 +314,7 @@ function executeFirebaseLiteTests() {
   });
 }
 
-function firebaseAppTests(testName, firebaseNamespaceFactory) {
+function firebaseAppTests(testName, firebaseNamespaceFactory): void {
   describe(testName, () => {
     let firebase: FirebaseNamespace;
 

packages/database/src/realtime/WebSocketConnection.ts

@@ -270,7 +270,7 @@ export class WebSocketConnection implements Transport {
     if (this.frames.length == this.totalFrames) {
       const fullMess = this.frames.join('');
       this.frames = null;
-      const jsonMess = jsonEval(fullMess);
+      const jsonMess = jsonEval(fullMess) as object;
 
       //handle the message
       this.onMessage(jsonMess);

packages/logger/.eslintrc.json

@@ -0,0 +1,6 @@
+{
+    "extends": "../../config/.eslintrc.json",
+    "parserOptions": {
+        "project": "tsconfig.json"
+    }
+}

packages/logger/index.ts

@@ -17,7 +17,7 @@
 
 import { instances, LogLevel } from './src/logger';
 
-export function setLogLevel(level: LogLevel) {
+export function setLogLevel(level: LogLevel): void {
   instances.forEach(inst => {
     inst.logLevel = level;
   });

packages/logger/package.json

@@ -10,9 +10,11 @@
     "dist"
   ],
   "scripts": {
+    "lint": "eslint -c .eslintrc.json '**/*.ts' --ignore-path '../../.gitignore'",
+    "lint:fix": "eslint --fix -c .eslintrc.json '**/*.ts' --ignore-path '../../.gitignore'",
     "build": "rollup -c",
     "dev": "rollup -c -w",
-    "test": "run-p test:browser test:node",
+    "test": "run-p lint test:browser test:node",
     "test:browser": "karma start --single-run",
     "test:browser:debug": "karma start --browsers Chrome --auto-watch",
     "test:node": "TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\"}' nyc --reporter lcovonly -- mocha test/**/*.test.* --opts ../../config/mocha.node.opts",
@@ -40,7 +42,11 @@
     "ts-loader": "5.4.5",
     "ts-node": "8.1.0",
     "typescript": "3.4.5",
-    "webpack": "4.30.0"
+    "webpack": "4.30.0",
+    "eslint": "5.16.0",
+    "@typescript-eslint/parser": "1.9.0",
+    "@typescript-eslint/eslint-plugin": "1.9.0",
+    "@typescript-eslint/eslint-plugin-tslint": "1.9.0"
   },
   "repository": {
     "directory": "packages/logger",