Skip to content

Commit bd61e90

Browse files
authored
Redesign SDK to get rid of "exports =" and export more types. (#80)
Redesign SDK to get rid of "exports =" and export more types. Using "exports =" means that clients are supposed to need import = require()[1]. This is a horrible anti-pattern that leaks to our customers so we should avoid it. This also tries to address outstanding concerns about how the SDK can be made extensible for other providers and how we will make sure all types customers need for production are exported in index.ts. To do this I tried to follow a few principles: 1. A provider's export is a merged namespace + function. This styles the Functions SDK after the core JS SDK. 2. All provider namespaces should have a type called <provider>.FunctionBuilder. This improves predictability 3. Since providers' types will be referenced from their namespace, they should avoid a redundant prefix. E.g. database.FunctionBuilder not database.DatabaseFunctionBuilder. 4. All services offer a singleton at service() implementing service.Service. The singleton must be initialized in index.ts with service.init()) 5. index.ts's purpose is to initialize services, rexport provider namespaces, and rexport loose/common types. I admit the result is more like weak binding than true dependency injection because the provider constructors depend on service singletons. The classes should all be DI friendly still for better testability. I'm increasingly uncomfortable with functions.env. Because env is a property bag we can't actually export env.Env. There's really no reasonable reason we would assume anyone can inject their own env type for testing. I think we may have to make this functions.env() which returns an env.Env (currently env.Data) or throws. I think env could be its own conversation and I'm OK if we have to wait until the new year for that. In the meantime, the only functional change is that the failure case of accessing functions.env too early changes from a helpful error message to a null pointer exception. [1]: https://www.typescriptlang.org/docs/handbook/modules.html
1 parent 9fe6137 commit bd61e90

36 files changed

+900
-1536
lines changed

package.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
"@types/sinon": "^1.16.29",
3434
"chai": "^3.5.0",
3535
"chai-as-promised": "^5.2.0",
36-
"firebase": "^3.3",
36+
"firebase-admin": "^4.0.5",
3737
"istanbul": "^0.4.2",
3838
"mocha": "^2.4.5",
3939
"nock": "^8.0.0",
@@ -42,12 +42,12 @@
4242
"typescript": "^2.0.0"
4343
},
4444
"peerDependencies": {
45-
"firebase": "^3.3"
45+
"firebase-admin": "^4.0.5"
4646
},
4747
"dependencies": {
4848
"@types/bluebird": "^3.0.32",
49-
"@types/jsonwebtoken": "^7.1.32",
5049
"@types/express": "^4.0.33",
50+
"@types/jsonwebtoken": "^7.1.32",
5151
"@types/lodash": "^4.14.34",
5252
"@types/node": "^6.0.38",
5353
"@types/request": "0.0.30",

spec/apps.spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import { expect } from 'chai';
22

33
import { FakeEnv } from './support/helpers';
4-
import Apps from '../src/apps';
5-
import * as firebase from 'firebase';
4+
import { apps as appsNamespace } from '../src/apps';
5+
import * as firebase from 'firebase-admin';
66

77
describe('apps', () => {
8-
let apps;
8+
let apps: appsNamespace.Apps;
99
beforeEach(() => {
10-
apps = new Apps(new FakeEnv());
10+
apps = new appsNamespace.Apps(new FakeEnv());
1111
});
1212

1313
it('should load the admin app for admin impersonation', function () {

spec/builders/database-builder.spec.ts

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

spec/credential.spec.ts

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

0 commit comments

Comments
 (0)