Modularize functions #3051

hsubox76 · 2020-05-11T23:17:01Z

First draft at modularizing firebase/functions.

packages-exp/functions-exp/package.json

packages-exp/functions-exp/src/api.ts

packages-exp/functions-exp/src/context.ts

packages-exp/functions-exp/src/error.ts

packages-exp/functions-exp/src/index.node.ts

packages-exp/functions-exp/src/service.ts

google-oss-bot · 2020-05-20T00:44:13Z

Binary Size Report

Affected SDKs

@firebase/util

Type Base (e4babee) Head (2c9044b) Diff

browser 19.6 kB 19.6 kB -3 B (-0.0%)

esm2017 17.5 kB 17.5 kB -3 B (-0.0%)

main 19.6 kB 19.6 kB -3 B (-0.0%)

module 18.7 kB 18.7 kB -3 B (-0.0%)

`firebase`

Click to show 14 binary size changes.

Type	Base (`e4babee`)	Head (`2c9044b`)	Diff
firebase-analytics.js	26.6 kB	26.6 kB	-3 B (-0.0%)
firebase-app.js	19.9 kB	19.9 kB	-3 B (-0.0%)
firebase-database.js	187 kB	187 kB	-3 B (-0.0%)
firebase-firestore.js	287 kB	287 kB	-3 B (-0.0%)
firebase-firestore.memory.js	226 kB	226 kB	-3 B (-0.0%)
firebase-functions.js	9.60 kB	9.60 kB	-3 B (-0.0%)
firebase-installations.js	19.2 kB	19.2 kB	-3 B (-0.0%)
firebase-messaging.js	39.1 kB	39.1 kB	-3 B (-0.0%)
firebase-performance-standalone.es2017.js	72.0 kB	72.0 kB	-3 B (-0.0%)
firebase-performance-standalone.js	47.2 kB	47.2 kB	-3 B (-0.0%)
firebase-performance.js	37.6 kB	37.6 kB	-3 B (-0.0%)
firebase-remote-config.js	37.0 kB	37.0 kB	-3 B (-0.0%)
firebase-storage.js	40.8 kB	40.8 kB	-3 B (-0.0%)
firebase.js	819 kB	819 kB	-3 B (-0.0%)

Test Logs

Base (e4babeee): https://github.com/firebase/firebase-js-sdk/actions/runs/141319070
Head (2c9044b4): https://github.com/firebase/firebase-js-sdk/actions/runs/141323091

Feiyang1 · 2020-05-20T18:19:52Z

packages-exp/functions-exp/.eslintrc.js

+      {
+        'packageDir': [path.resolve(__dirname, '../../'), './'],
+        devDependencies: true,
+        peerDependencies: true


Shouldn't peerDependencies to be allowed?

This allows peerDependencies (will not show error on peerDependency import). Are you saying it should not be allowed?

Also this whole rule is a mess for monorepos, it looks like someone has submitted a PR to try to fix it but it has not been looked at yet: import-js/eslint-plugin-import#1650

Right. I meant not allowed.

Removed. I thought it would give me errors on imports from @firebase/app-types-exp but I guess it doesn't? I don't know how this rule works anymore.

packages-exp/functions-exp/package.json

packages-exp/functions-exp/src/callable.test.ts

packages-exp/functions-exp/src/service.ts

packages-exp/functions-exp/test/utils.ts

packages-exp/functions-exp/src/error.ts

Feiyang1 · 2020-06-05T19:20:11Z

packages-exp/functions-exp/src/error.ts

+     * A standard error code that will be returned to the client. This also
+     * determines the HTTP status code of the response, as defined in code.proto.
+     */
+    readonly code: FunctionsErrorCode,


It should be a simple input parameter without readonly annotation because otherwise it would create a property code on FunctionsError.

Feiyang1 · 2020-06-05T19:21:05Z

packages-exp/functions-exp/src/error.ts

+     */
+    readonly details?: unknown
+  ) {
+    super(code, message || '');


The format for code shoud be service/error-code-string

Feiyang1 · 2020-06-05T19:24:04Z

packages-exp/functions-exp/src/service.ts

+ * The main class for the Firebase Functions SDK.
+ * @internal
+ */
+export class FunctionsService {


Can you make it extend https://github.com/firebase/firebase-js-sdk/blob/master/packages-exp/app-types-exp/index.d.ts#L110 ?

Feiyang1 · 2020-06-16T17:45:49Z

packages/util/src/errors.ts

@@ -66,7 +66,8 @@ export interface StringLike {
 }

 export interface ErrorData {
-  [key: string]: StringLike | undefined;
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  [key: string]: any;


Can it be unknown?

Yes, but to get around an error in line 151 I had to change .toString() to String(), hopefully that's ok.

Feiyang1 · 2020-06-16T17:49:11Z

scripts/exp/update-internal-dep-versions.js

+async function updateField(pkg, fieldName) {
+  const field = pkg[fieldName];
+  for (const depName in field) {
+    if (!depName.includes('@firebase')) continue;


We should update the firebase package as well as packages may depend on it, e.g.@firebase/testing and rxFire

hsubox76 · 2020-06-18T20:47:20Z

I'm going to wait for this week's release to be merged to master, rebase, run package.json version update script, and then merge.

Feiyang1 · 2020-06-20T22:14:51Z

packages-exp/functions-types-exp/package.json

@@ -0,0 +1,25 @@
+{
+  "name": "@firebase/functions-types-exp",


Can you please make it private?

hsubox76 requested review from Feiyang1 and hiranya911 as code owners May 11, 2020 23:17

Feiyang1 reviewed May 12, 2020

View reviewed changes

hsubox76 force-pushed the ch-functionsnext branch from d2243fa to 0526724 Compare May 19, 2020 23:59

Feiyang1 reviewed May 20, 2020

View reviewed changes

hsubox76 force-pushed the ch-functionsnext branch from c5ea05e to 8189fe2 Compare May 29, 2020 20:39

hsubox76 assigned Feiyang1 Jun 1, 2020

hsubox76 force-pushed the ch-functionsnext branch from 8189fe2 to ce8fa07 Compare June 3, 2020 00:43

Feiyang1 suggested changes Jun 5, 2020

View reviewed changes

Feiyang1 assigned hsubox76 and unassigned Feiyang1 Jun 5, 2020

hsubox76 force-pushed the ch-functionsnext branch from ce8fa07 to b6667e9 Compare June 11, 2020 22:23

Feiyang1 approved these changes Jun 16, 2020

View reviewed changes

hsubox76 added 12 commits June 19, 2020 14:41

Rough implementation

d4a0bb5

Address PR comments

a009aed

Update out-of-sync internal versions

6a89dbd

Fix build issues.

d5f161a

Add test:ci script to functions-types-exp

8fa62a5

Address PR comments

26b0255

Address PR comments

47a1c9b

Update package.json versions to fix build

1b4d1ff

Address PR comments

5ee7d4f

Explanatory comments for utility script

2da83d1

Address PR comments

441ac6a

Update package.json versions

ce280e5

hsubox76 force-pushed the ch-functionsnext branch from b6667e9 to ce280e5 Compare June 19, 2020 21:43

hsubox76 merged commit d470f49 into master Jun 20, 2020

Feiyang1 reviewed Jun 20, 2020

View reviewed changes

hsubox76 mentioned this pull request Jun 23, 2020

Set functions-types-exp package to private #3262

Merged

firebase locked and limited conversation to collaborators Jul 21, 2020

hsubox76 changed the title ~~WIP: Modularize functions~~ Modularize functions Aug 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modularize functions #3051

Modularize functions #3051

hsubox76 commented May 11, 2020

google-oss-bot commented May 20, 2020 •

edited

`@firebase/util`

`firebase`

Feiyang1 May 20, 2020

hsubox76 May 20, 2020

Feiyang1 May 29, 2020

hsubox76 May 29, 2020

Feiyang1 Jun 5, 2020

hsubox76 Jun 11, 2020

Feiyang1 Jun 5, 2020

hsubox76 Jun 11, 2020

Feiyang1 Jun 5, 2020

hsubox76 Jun 11, 2020

Feiyang1 Jun 16, 2020

hsubox76 Jun 18, 2020

Feiyang1 Jun 16, 2020

hsubox76 Jun 18, 2020

hsubox76 commented Jun 18, 2020

Feiyang1 Jun 20, 2020

Modularize functions #3051

Modularize functions #3051

Conversation

hsubox76 commented May 11, 2020

google-oss-bot commented May 20, 2020 • edited

Binary Size Report

Affected SDKs

@firebase/util

firebase

Test Logs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hsubox76 commented Jun 18, 2020

Choose a reason for hiding this comment

google-oss-bot commented May 20, 2020 •

edited

`@firebase/util`

`firebase`