Skip to content

Apply strictNullChecks to all packages except database #1805

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 23 commits into from
Jun 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
21e14c0
TS changes before splitting database
hsubox76 May 10, 2019
9f38d85
Roll back database changes
hsubox76 May 10, 2019
10fe94e
[AUTOMATED]: Prettier Code Styling
hsubox76 May 10, 2019
e9d9a93
Put back database nullcheck exception
hsubox76 May 10, 2019
263cd24
Finish fixing strictNullCheck errors
hsubox76 May 20, 2019
54bdc4e
Fix logger test.
hsubox76 May 20, 2019
e1ab520
Merge branch 'master' of https://github.com/firebase/firebase-js-sdk …
hsubox76 May 20, 2019
f246e21
Fix tests
hsubox76 May 21, 2019
3e1ea9b
Update primitive array types to type[]
hsubox76 May 21, 2019
8025369
[AUTOMATED]: Prettier Code Styling
hsubox76 May 21, 2019
6e668c0
Put test exclusion back in rxfire
hsubox76 May 21, 2019
ce0118b
Address PR comments
hsubox76 May 22, 2019
2e41a2f
[AUTOMATED]: Prettier Code Styling
hsubox76 May 22, 2019
95aa743
Addressing PR comments
hsubox76 May 22, 2019
1c5b5c5
Few more comments to address
hsubox76 May 23, 2019
9c3d52f
change assertThrows to assume it never returns null
hsubox76 May 28, 2019
6ec6881
[AUTOMATED]: Prettier Code Styling
hsubox76 May 28, 2019
384c580
Merge branch 'master' of https://github.com/firebase/firebase-js-sdk …
hsubox76 May 29, 2019
aca9dd8
Fix additional warnings after merge
hsubox76 May 29, 2019
88f27cf
Fix storage test
hsubox76 May 29, 2019
d9c9040
[AUTOMATED]: Prettier Code Styling
hsubox76 May 29, 2019
846f100
Address comments in PR
hsubox76 May 31, 2019
df401be
Merge branch 'master' of https://github.com/firebase/firebase-js-sdk …
hsubox76 May 31, 2019
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
1 change: 1 addition & 0 deletions config/tsconfig.base.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"compilerOptions": {
"declaration": true,
"importHelpers": true,
"strictNullChecks": true,
"lib": [
"es2015",
"dom"
Expand Down
2 changes: 1 addition & 1 deletion integration/typescript/test/namespace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import * as firebase from 'firebase';
import * as namespaceDefinition from '../../shared/namespaceDefinition.json';
import * as validateNamespace from '../../shared/validator';
import validateNamespace from '../../shared/validator';

firebase.initializeApp({
apiKey: 'test-api-key',
Expand Down
3 changes: 2 additions & 1 deletion integration/typescript/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"noImplicitAny": true,
"outDir": "dist",
"target": "ES5",
"sourceMap": true
"sourceMap": true,
"esModuleInterop": true
},
"exclude": [
"node_modules",
Expand Down
6 changes: 3 additions & 3 deletions packages/app/src/firebaseApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ export class FirebaseAppImpl implements FirebaseApp {
}

return Promise.all(
services.map(service => {
return service.INTERNAL.delete();
})
services
.filter(service => 'INTERNAL' in service)
.map(service => service.INTERNAL!.delete())
);
})
.then(
Expand Down
6 changes: 3 additions & 3 deletions packages/app/src/lite/firebaseAppLite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@ export class FirebaseAppLiteImpl implements FirebaseApp {
}

return Promise.all(
services.map(service => {
return service.INTERNAL.delete();
})
services
.filter(service => 'INTERNAL' in service)
.map(service => service.INTERNAL!.delete())
);
})
.then(
Expand Down
14 changes: 8 additions & 6 deletions packages/app/test/firebaseApp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ function executeFirebaseTests() {
const [app, , instanceIdentifier] = args;
return new TestService(app, instanceIdentifier);
},
null,
null,
undefined,
undefined,
true
);
firebase.initializeApp({});
Expand Down Expand Up @@ -200,8 +200,8 @@ function executeFirebaseTests() {
const [app, , instanceIdentifier] = args;
return new TestService(app, instanceIdentifier);
},
null,
null,
undefined,
undefined,
false // <-- multi instance flag
);
firebase.initializeApp({});
Expand Down Expand Up @@ -268,11 +268,13 @@ function executeFirebaseTests() {
// service's value.
return (app as _FirebaseApp).INTERNAL.getToken()
.then(token => {
assert.equal('tokenFor0', token.accessToken);
assert.isNotNull(token);
assert.equal('tokenFor0', token!.accessToken);
return (app2 as _FirebaseApp).INTERNAL.getToken();
})
.then(token => {
assert.equal('tokenFor1', token.accessToken);
assert.isNotNull(token);
assert.equal('tokenFor1', token!.accessToken);
});
});
});
Expand Down
3 changes: 2 additions & 1 deletion packages/database/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
{
"extends": "../../config/tsconfig.base.json",
"compilerOptions": {
"outDir": "dist"
"outDir": "dist",
"strictNullChecks": false
},
"exclude": [
"dist/**/*"
Expand Down
2 changes: 1 addition & 1 deletion packages/functions/test/callable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe('Firebase Functions > Call', () => {
functions = new Service(app, region);
if (useEmulator) {
functions.useFunctionsEmulator(
process.env.FIREBASE_FUNCTIONS_EMULATOR_ORIGIN
process.env.FIREBASE_FUNCTIONS_EMULATOR_ORIGIN!
);
}
});
Expand Down
1 change: 0 additions & 1 deletion packages/functions/test/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ describe('Firebase Functions > Service', () => {
service._url('foo'),
'http://localhost:5005/my-project/us-central1/foo'
);
service.useFunctionsEmulator(null);
});
});

Expand Down
21 changes: 9 additions & 12 deletions packages/logger/test/logger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,28 @@
*/

import { expect } from 'chai';
import { spy as Spy } from 'sinon';
import { spy as Spy, SinonSpy } from 'sinon';
import { Logger, LogLevel } from '../src/logger';
import { setLogLevel } from '../index';
import { debug } from 'util';

describe('@firebase/logger', () => {
const message = 'Hello there!';
let client: Logger;
const spies = {
logSpy: null,
infoSpy: null,
warnSpy: null,
errorSpy: null
};
let spies: { [key: string]: SinonSpy };
/**
* Before each test, instantiate a new instance of Logger and establish spies
* on all of the console methods so we can assert against them as needed
*/
beforeEach(() => {
client = new Logger('@firebase/test-logger');

spies.logSpy = Spy(console, 'log');
spies.infoSpy = Spy(console, 'info');
spies.warnSpy = Spy(console, 'warn');
spies.errorSpy = Spy(console, 'error');
spies = {
logSpy: Spy(console, 'log'),
infoSpy: Spy(console, 'info'),
warnSpy: Spy(console, 'warn'),
errorSpy: Spy(console, 'error')
};
});

afterEach(() => {
Expand All @@ -62,7 +59,7 @@ describe('@firebase/logger', () => {
} call \`console.${channel}\` if \`.${channel}\` is called`, () => {
client[channel](message);
expect(
spies[`${channel}Spy`] && spies[`${channel}Spy`].called,
spies[`${channel}Spy`]!.called,
`Expected ${channel} to ${shouldLog ? '' : 'not'} log`
).to.be[shouldLog ? 'true' : 'false'];
});
Expand Down
14 changes: 8 additions & 6 deletions packages/rxfire/database/list/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import { database } from 'firebase';
import { QueryChange, ListenEvent } from '../interfaces';
import { Observable, of, merge, from } from 'rxjs';
import { validateEventsArray, isNil } from '../utils';
import { validateEventsArray } from '../utils';
import { fromRef } from '../fromRef';
import { switchMap, scan, distinctUntilChanged, map } from 'rxjs/operators';
import { changeToData } from '../object';
Expand All @@ -42,11 +42,13 @@ export function list(
query: database.Query,
events?: ListenEvent[]
): Observable<QueryChange[]> {
events = validateEventsArray(events);
const eventsList = validateEventsArray(events);
return fromOnce(query).pipe(
switchMap(change => {
const childEvent$ = [of(change)];
events.forEach(event => childEvent$.push(fromRef(query, event)));
for (const event of eventsList) {
childEvent$.push(fromRef(query, event));
}
return merge(...childEvent$).pipe(scan(buildView, []));
}),
distinctUntilChanged()
Expand Down Expand Up @@ -78,7 +80,7 @@ function positionFor(changes: QueryChange[], key) {
}

function positionAfter(changes: QueryChange[], prevKey?: string) {
if (isNil(prevKey)) {
if (prevKey == null) {
return 0;
} else {
const i = positionFor(changes, prevKey);
Expand All @@ -94,11 +96,11 @@ function buildView(current: QueryChange[], change: QueryChange) {
const { snapshot, prevKey, event } = change;
const { key } = snapshot;
const currentKeyPosition = positionFor(current, key);
const afterPreviousKeyPosition = positionAfter(current, prevKey);
const afterPreviousKeyPosition = positionAfter(current, prevKey || undefined);
switch (event) {
case ListenEvent.value:
if (change.snapshot && change.snapshot.exists()) {
let prevKey = null;
let prevKey: string | null = null;
change.snapshot.forEach(snapshot => {
const action: QueryChange = {
snapshot,
Expand Down
8 changes: 2 additions & 6 deletions packages/rxfire/database/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,13 @@

import { ListenEvent } from './interfaces';

export function isNil(obj: any): boolean {
return obj === undefined || obj === null;
}

/**
* Check the length of the provided array. If it is empty return an array
* that is populated with all the Realtime Database child events.
* @param events
*/
export function validateEventsArray(events?: ListenEvent[]) {
if (isNil(events) || events!.length === 0) {
export function validateEventsArray(events?: ListenEvent[]): ListenEvent[] {
if (events == null || events.length === 0) {
events = [
ListenEvent.added,
ListenEvent.removed,
Expand Down
2 changes: 1 addition & 1 deletion packages/rxfire/firestore/collection/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const filterEvents = (events?: firestore.DocumentChangeType[]) =>
let hasChange = false;
for (let i = 0; i < changes.length; i++) {
const change = changes[i];
if (events.indexOf(change.type) >= 0) {
if (events && events.indexOf(change.type) >= 0) {
hasChange = true;
break;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/rxfire/firestore/document/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export function docData<T>(
}

export function snapToData(
snapshot: firestore.QueryDocumentSnapshot,
snapshot: firestore.DocumentSnapshot,
idField?: string
) {
return {
Expand Down
10 changes: 5 additions & 5 deletions packages/rxfire/test/database.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ let batch = (items: any[]) => {
};

describe('RxFire Database', () => {
let app: app.App = null;
let database: database.Database = null;
let app: app.App;
let database: database.Database;
let ref = (path: string) => {
app.database().goOffline();
return app.database().ref(path);
app!.database().goOffline();
return app!.database().ref(path);
};

function prepareList(
Expand Down Expand Up @@ -147,7 +147,7 @@ describe('RxFire Database', () => {
count = count + 1;
const { event, snapshot } = change;
expect(event).to.equal(ListenEvent.added);
expect(snapshot.val()).to.eql(data[snapshot.key]);
expect(snapshot.val()).to.eql(data[snapshot.key!]);
if (count === items.length) {
done();
sub.unsubscribe();
Expand Down
4 changes: 2 additions & 2 deletions packages/rxfire/test/firestore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ const seedTest = firestore => {
};

describe('RxFire Firestore', () => {
let app: app.App = null;
let firestore: firestore.Firestore = null;
let app: app.App;
let firestore: firestore.Firestore;

/**
* Each test runs inside it's own app instance and the app
Expand Down
9 changes: 7 additions & 2 deletions packages/storage/src/implementation/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { Location } from './location';
import * as json from './json';
import * as type from './type';
import { ListResult } from '../list';
import * as errors from './error';

/**
* Represents the simplified object metadata returned by List API.
Expand Down Expand Up @@ -58,11 +59,15 @@ function fromBackendResponse(
items: [],
nextPageToken: resource['nextPageToken']
};
const bucket = authWrapper.bucket();
if (bucket === null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is why I don't like mixing ==null in the code. I have to look up authWrapper.bucket() signature to understand if it is actually meant to be == and made a mistake.

throw errors.noDefaultBucket();
}
if (resource[PREFIXES_KEY]) {
for (const path of resource[PREFIXES_KEY]) {
const pathWithoutTrailingSlash = path.replace(/\/$/, '');
const reference = authWrapper.makeStorageReference(
new Location(authWrapper.bucket(), pathWithoutTrailingSlash)
new Location(bucket, pathWithoutTrailingSlash)
);
listResult.prefixes.push(reference);
}
Expand All @@ -71,7 +76,7 @@ function fromBackendResponse(
if (resource[ITEMS_KEY]) {
for (const item of resource[ITEMS_KEY]) {
const reference = authWrapper.makeStorageReference(
new Location(authWrapper.bucket(), item['name'])
new Location(bucket, item['name'])
);
listResult.items.push(reference);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/storage/src/implementation/location.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export class Location {
}

static makeFromUrl(url: string): Location {
let location = null;
let location: Location | null = null;
let bucketDomain = '([A-Za-z0-9.\\-_]+)';

function gsModify(loc: Location) {
Expand Down
2 changes: 1 addition & 1 deletion packages/storage/src/implementation/metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export function getMappings(): Mappings {
if (mappings_) {
return mappings_;
}
let mappings = [];
const mappings: Mapping[] = [];
mappings.push(new Mapping('bucket'));
mappings.push(new Mapping('generation'));
mappings.push(new Mapping('metageneration'));
Expand Down
17 changes: 9 additions & 8 deletions packages/storage/src/implementation/observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,17 @@
* limitations under the License.
*/
import * as type from './type';
import { FirebaseStorageError } from './error';

type NextFn<T> = (value: T) => void;
type ErrorFn = (error: Error) => void;
type ErrorFn = (error: Error | FirebaseStorageError) => void;
type CompleteFn = () => void;
type Unsubscribe = () => void;

type Subscribe<T> = (
next: NextFn<T> | { [name: string]: string | null },
error?: ErrorFn,
complete?: CompleteFn
next?: NextFn<T> | { [name: string]: string | null } | null,
error?: ErrorFn | null,
complete?: CompleteFn | null
) => Unsubscribe;

export { NextFn, ErrorFn, CompleteFn, Unsubscribe, Subscribe };
Expand All @@ -33,12 +34,12 @@ export { NextFn, ErrorFn, CompleteFn, Unsubscribe, Subscribe };
* @struct
*/
export class Observer<T> {
next: NextFn<T> | null;
error: ErrorFn | null;
complete: CompleteFn | null;
next?: NextFn<T> | null;
error?: ErrorFn | null;
complete?: CompleteFn | null;

constructor(
nextOrObserver: NextFn<T> | { [name: string]: string | null } | null,
nextOrObserver?: NextFn<T> | { [name: string]: string | null } | null,
opt_error?: ErrorFn | null,
opt_complete?: CompleteFn | null
) {
Expand Down
Loading