Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jun 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion config/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
"**/test/**/*.ts"
],
"rules": {
"no-unused-expressions": "off"
"no-unused-expressions": "off",
"@typescript-eslint/no-explicit-any": "off"
}
}
],
Expand Down
1 change: 1 addition & 0 deletions packages/app/index.rn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions packages/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(`
Expand Down
2 changes: 1 addition & 1 deletion packages/app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion packages/app/src/firebaseApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions packages/app/src/firebaseNamespace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
};

/**
Expand Down
35 changes: 18 additions & 17 deletions packages/app/src/firebaseNamespaceCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand All @@ -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', {
Expand All @@ -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).
*/
Expand All @@ -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 };
Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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
});
}

Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/lite/firebaseAppLite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
8 changes: 5 additions & 3 deletions packages/app/src/lite/firebaseNamespaceLite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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`);
Expand Down
24 changes: 11 additions & 13 deletions packages/app/test/firebaseApp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { assert } from 'chai';
executeFirebaseTests();
executeFirebaseLiteTests();

function executeFirebaseTests() {
function executeFirebaseTests(): void {
firebaseAppTests('Firebase App Tests', createFirebaseNamespace);

describe('Firebase Service Registration', () => {
Expand All @@ -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();
});

Expand Down Expand Up @@ -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) {
Expand All @@ -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();
});

Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -280,7 +279,7 @@ function executeFirebaseTests() {
});
}

function executeFirebaseLiteTests() {
function executeFirebaseLiteTests(): void {
firebaseAppTests('Firebase App Lite Tests', createFirebaseNamespaceLite);

describe('Firebase Lite Service Registration', () => {
Expand All @@ -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);
});
Expand All @@ -316,7 +314,7 @@ function executeFirebaseLiteTests() {
});
}

function firebaseAppTests(testName, firebaseNamespaceFactory) {
function firebaseAppTests(testName, firebaseNamespaceFactory): void {
describe(testName, () => {
let firebase: FirebaseNamespace;

Expand Down
2 changes: 1 addition & 1 deletion packages/database/src/realtime/WebSocketConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions packages/logger/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"extends": "../../config/.eslintrc.json",
"parserOptions": {
"project": "tsconfig.json"
}
}
2 changes: 1 addition & 1 deletion packages/logger/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand Down
10 changes: 8 additions & 2 deletions packages/logger/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
Loading