Add API Extractor for @firestore/lite + exp #3657

schmidt-sebastian · 2020-08-18T21:25:22Z

This adds API Extractor to firestore lite. Fingers crossed that we can get the generated and the automated types to match (via follow-up cleanup).

changeset-bot · 2020-08-18T21:25:24Z

💥 No Changeset

Latest commit: 37c864b

Merging this PR will not cause any packages to be released. If these changes should not cause updates to packages in this repo, this is fine 🙂

If these changes should be published to npm, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

schmidt-sebastian · 2020-08-18T21:32:47Z

packages/firestore/lite/package.json

@@ -1,5 +1,5 @@
 {
-  "name": "@firebase/firestore/lite",
+  "name": "@firebase/firestore-lite",


API Extractor considers this name invalid - looks like one slash is the maximum.

Api-extractor doesn't support multiple entry points, so you need to trick it to think that lite is a package instead of a submodule.

(I have a PR to support multiple entry points, but only for documentation and api report)

This name is invalid even outside of this PR. I get this error from IntelliJ when I turn on JSON schema validation:

String violates the pattern: '^(?:@[a-z0-9-*~][a-z0-9-*._~]*/)?[a-z0-9-~][a-z0-9-._~]*$' Inspection info: This inspection checks that JSON files conform to JSON Schemas assigned to them

The pattern seems to allow only one scoped package. Do we need to change it or should I spend some time trying to figure out how to work around this limitation without the name change?

The name actually doesn't matter, so it can be whatever that's easier for you. We use/ to be consistent with how developer would use the submodules, so I prefer to stay this way. Does it give you any issue other than unpleasant validation error?

I get the following error:

Error: Error parsing /Users/mrschmidt/GitHub/firebase/firebase-js-sdk/packages/firestore/lite/api-extractor.json: The package name "@firebase/firestore/lite" contains an invalid character: "/"

The build stops after this message.

oh right, api-extractor definitely doesn't like it. You can change it to - to work around it for now. I'm planning to release a forked version of api-extractor with support for multiple entry points. We will come back to this once that's done.

schmidt-sebastian · 2020-08-18T21:33:52Z

package.json

@@ -136,6 +135,7 @@
    "ora": "4.0.5",
    "prettier": "2.0.5",
    "protractor": "5.4.2",
+    "rollup-plugin-copy": "3.3.0",


rollup-plugin-copy-asset doesn't allow me to move the proto files to dist/lite/src/proto

schmidt-sebastian · 2020-08-18T21:38:14Z

package.json

@@ -66,8 +66,7 @@
    "@changesets/changelog-github": "0.2.6",
    "@changesets/cli": "2.9.2",
    "@microsoft/api-documenter": "7.8.21",
-    "@microsoft/api-extractor": "7.9.2",
-    "@types/mz": "2.7.1",
+    "@microsoft/api-extractor": "7.8.2-pr1796.0",


This pulls in microsoft/rushstack#1029, which makes this diff about 500 lines smaller (see the revert 609b2b3)

eh, the commit shows 445 additions and 477 deletions, so the net gain seems to be 32 lines?

Yeah, it 500 lines in terms of changed lines. I can go with the version that doesn't require the custom release if you prefer that (which might make sense especially since we seem to waiting on a lot of other PRs).

FWIW, I created a new PR to merge just this: #3660

schmidt-sebastian · 2020-08-18T21:38:30Z

package.json

@@ -66,8 +66,7 @@
    "@changesets/changelog-github": "0.2.6",
    "@changesets/cli": "2.9.2",
    "@microsoft/api-documenter": "7.8.21",
-    "@microsoft/api-extractor": "7.9.2",
-    "@types/mz": "2.7.1",


This is in here twice.

google-oss-bot · 2020-08-18T21:44:10Z

Binary Size Report

Affected SDKs

@firebase/firestore-exp

Type Base (3b26db4) Head (73b9fa9) Diff

browser ? 189 kB ? (?)

main ? 468 kB ? (?)

module ? 189 kB ? (?)

react-native ? 189 kB ? (?)
@firebase/firestore-lite

Type Base (3b26db4) Head (73b9fa9) Diff

browser ? 64.8 kB ? (?)

main ? 141 kB ? (?)

module ? 64.8 kB ? (?)

react-native ? 64.9 kB ? (?)

Test Logs

Base (3b26db42): https://github.com/firebase/firebase-js-sdk/actions/runs/216842155
Head (73b9fa99): https://github.com/firebase/firebase-js-sdk/actions/runs/217460034

Feiyang1

Are you planning to do the same for exp?

Feiyang1 · 2020-08-18T23:15:51Z

packages/firestore/lite/api-extractor.json

+  "extends": "../../../config/api-extractor.json",
+  "mainEntryPointFilePath": "../dist/lite/firestore/lite/index.d.ts",
+  "dtsRollup": {
+    "enabled": true,


Do you think you will ever need main and lite to share declarations? There is a risk where the same type from main and lite becomes incompatible if they use separate rollup d.ts files - see discussion here. It's probably not a problem for us, since we are already using separate d.ts files for lite and main, but it's good to keep that in mind.

I don't think we want to do that, neither now nor in the future. Most of our types are incompatible with one another (i.e. a DocumentReference that contains a a Lite Firestore is incompatible with a DocumentReference for the main Firestore).

Feiyang1 · 2020-08-18T23:33:21Z

package.json

@@ -66,8 +66,7 @@
    "@changesets/changelog-github": "0.2.6",
    "@changesets/cli": "2.9.2",
    "@microsoft/api-documenter": "7.8.21",
-    "@microsoft/api-extractor": "7.9.2",
-    "@types/mz": "2.7.1",
+    "@microsoft/api-extractor": "7.8.2-pr1796.0",


eh, the commit shows 445 additions and 477 deletions, so the net gain seems to be 32 lines?

Feiyang1 · 2020-08-18T23:35:30Z

packages/firestore/lite/package.json

@@ -1,5 +1,5 @@
 {
-  "name": "@firebase/firestore/lite",
+  "name": "@firebase/firestore-lite",


Api-extractor doesn't support multiple entry points, so you need to trick it to think that lite is a package instead of a submodule.

(I have a PR to support multiple entry points, but only for documentation and api report)

Feiyang1 · 2020-08-18T23:43:35Z

packages/firestore/package.json

@@ -7,14 +7,15 @@
  "description": "The Cloud Firestore component of the Firebase JS SDK.",
  "author": "Firebase <[email protected]> (https://firebase.google.com/)",
  "scripts": {
+    "api-report:lite": "(cd lite; api-extractor run --local --verbose)",


Once this PR is merged, we can generate api reports for multiple entry points together, so you won't generate report for lite and main separately.
It seems it might take a long time to merge though, so I might publish a forked version to npm which we will use. For now, this is okay.

Feiyang1 · 2020-08-18T23:46:09Z

packages/firestore/rollup.config.lite.js

+        dest: 'dist/lite/src'
+      },
+      {
+        // Copy into generated source files to support API Extractor


Why does API extractor need a different path?

The source tree that it uses are the .d.ts generated files in dist/lite/firestore/src, and by default the "proto" dir is missing. I assume that is because we provide d.ts files to begin with and so nothing gets generated.

FWIW, I changed this to only copy the d.ts file.

schmidt-sebastian

Some follow up questions.

I planned to do this in a follow-up to for /exp. I realized that we are not actually blocked because of the behavior differences that I would like to clarify in the API documentation, since the APIs that have different behavior are already implemented twice (e.g. getDoc). I pushed the changes to this PR now since without them the build was broken.

schmidt-sebastian · 2020-08-19T11:37:29Z

package.json

@@ -66,8 +66,7 @@
    "@changesets/changelog-github": "0.2.6",
    "@changesets/cli": "2.9.2",
    "@microsoft/api-documenter": "7.8.21",
-    "@microsoft/api-extractor": "7.9.2",
-    "@types/mz": "2.7.1",
+    "@microsoft/api-extractor": "7.8.2-pr1796.0",


Yeah, it 500 lines in terms of changed lines. I can go with the version that doesn't require the custom release if you prefer that (which might make sense especially since we seem to waiting on a lot of other PRs).

schmidt-sebastian · 2020-08-19T11:40:36Z

packages/firestore/lite/api-extractor.json

+  "extends": "../../../config/api-extractor.json",
+  "mainEntryPointFilePath": "../dist/lite/firestore/lite/index.d.ts",
+  "dtsRollup": {
+    "enabled": true,


I don't think we want to do that, neither now nor in the future. Most of our types are incompatible with one another (i.e. a DocumentReference that contains a a Lite Firestore is incompatible with a DocumentReference for the main Firestore).

schmidt-sebastian · 2020-08-19T11:49:17Z

packages/firestore/lite/package.json

@@ -1,5 +1,5 @@
 {
-  "name": "@firebase/firestore/lite",
+  "name": "@firebase/firestore-lite",


This name is invalid even outside of this PR. I get this error from IntelliJ when I turn on JSON schema validation:

String violates the pattern: '^(?:@[a-z0-9-*~][a-z0-9-*._~]*/)?[a-z0-9-~][a-z0-9-._~]*$' Inspection info: This inspection checks that JSON files conform to JSON Schemas assigned to them

The pattern seems to allow only one scoped package. Do we need to change it or should I spend some time trying to figure out how to work around this limitation without the name change?

schmidt-sebastian · 2020-08-19T11:52:40Z

packages/firestore/rollup.config.lite.js

+        dest: 'dist/lite/src'
+      },
+      {
+        // Copy into generated source files to support API Extractor


The source tree that it uses are the .d.ts generated files in dist/lite/firestore/src, and by default the "proto" dir is missing. I assume that is because we provide d.ts files to begin with and so nothing gets generated.

FWIW, I changed this to only copy the d.ts file.

schmidt-sebastian · 2020-08-19T12:39:56Z

packages/firestore/lite-types/index.d.ts

@@ -365,31 +365,6 @@ export function snapshotEqual<T>(
  right: DocumentSnapshot<T> | QuerySnapshot<T>
 ): boolean;

-export type FirestoreErrorCode =


The Lite SDK doesn't use FirestoreError.

schmidt-sebastian · 2020-08-20T13:48:24Z

PR updated to undo the api-extractor upgrade.

schmidt-sebastian · 2020-09-16T21:15:04Z

Abandoning.

schmidt-sebastian added 2 commits August 18, 2020 23:14

Add Api Extractor for @firestore/lite

1dfec9a

Fix

6c129bf

schmidt-sebastian requested review from Feiyang1, hiranya911 and hsubox76 as code owners August 18, 2020 21:25

schmidt-sebastian commented Aug 18, 2020

View reviewed changes

schmidt-sebastian changed the title ~~Add Api Extractor for @firestore/lite~~ Add API Extractor for @firestore/lite Aug 18, 2020

schmidt-sebastian commented Aug 18, 2020

View reviewed changes

Undo namespace changes

71790fe

schmidt-sebastian commented Aug 18, 2020

View reviewed changes

schmidt-sebastian assigned Feiyang1 Aug 18, 2020

schmidt-sebastian added the api: firestore label Aug 18, 2020

Lint

bedb8d9

schmidt-sebastian force-pushed the mrschmidt/litetypes branch from 609b2b3 to bedb8d9 Compare August 18, 2020 21:52

Feiyang1 reviewed Aug 18, 2020

View reviewed changes

schmidt-sebastian added 3 commits August 19, 2020 14:07

Fix rollup config

5cbf013

Update exp as well

c7c4779

Add exp configuration

032b860

schmidt-sebastian commented Aug 19, 2020

View reviewed changes

schmidt-sebastian added 2 commits August 19, 2020 14:37

Add API review file

f4dfec7

Add TODO

81729b8

schmidt-sebastian commented Aug 19, 2020

View reviewed changes

schmidt-sebastian changed the title ~~Add API Extractor for @firestore/lite~~ Add API Extractor for @firestore/lite + exp Aug 19, 2020

schmidt-sebastian mentioned this pull request Aug 19, 2020

Remove all namespace imports #3660

Merged

schmidt-sebastian added 2 commits August 20, 2020 15:43

Merge

7e3f11f

Cleanup

a576652

schmidt-sebastian added 2 commits August 20, 2020 19:23

Fix merge

2e6b656

Remove no-op interface imports

37c864b

schmidt-sebastian closed this Sep 16, 2020

firebase locked and limited conversation to collaborators Oct 17, 2020

schmidt-sebastian deleted the mrschmidt/litetypes branch November 9, 2020 22:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add API Extractor for @firestore/lite + exp #3657

Add API Extractor for @firestore/lite + exp #3657

schmidt-sebastian commented Aug 18, 2020 •

edited

Loading

changeset-bot bot commented Aug 18, 2020 •

edited

Loading

schmidt-sebastian Aug 18, 2020

Feiyang1 Aug 18, 2020

schmidt-sebastian Aug 19, 2020 •

edited

Loading

Feiyang1 Aug 19, 2020

schmidt-sebastian Aug 19, 2020

Feiyang1 Aug 19, 2020

schmidt-sebastian Aug 18, 2020

schmidt-sebastian Aug 18, 2020

Feiyang1 Aug 18, 2020

schmidt-sebastian Aug 19, 2020

schmidt-sebastian Aug 19, 2020

schmidt-sebastian Aug 18, 2020

google-oss-bot commented Aug 18, 2020 •

edited

Loading

`@firebase/firestore-exp`

`@firebase/firestore-lite`

Feiyang1 left a comment

Feiyang1 Aug 18, 2020

schmidt-sebastian Aug 19, 2020

Feiyang1 Aug 18, 2020

Feiyang1 Aug 18, 2020

Feiyang1 Aug 18, 2020

Feiyang1 Aug 18, 2020

schmidt-sebastian Aug 19, 2020

schmidt-sebastian left a comment

schmidt-sebastian Aug 19, 2020

schmidt-sebastian Aug 19, 2020

schmidt-sebastian Aug 19, 2020 •

edited

Loading

schmidt-sebastian Aug 19, 2020

schmidt-sebastian Aug 19, 2020

schmidt-sebastian commented Aug 20, 2020

schmidt-sebastian commented Sep 16, 2020

Add API Extractor for @firestore/lite + exp #3657

Add API Extractor for @firestore/lite + exp #3657

Conversation

schmidt-sebastian commented Aug 18, 2020 • edited Loading

changeset-bot bot commented Aug 18, 2020 • edited Loading

💥 No Changeset

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schmidt-sebastian Aug 19, 2020 • edited Loading

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

google-oss-bot commented Aug 18, 2020 • edited Loading

Binary Size Report

Affected SDKs

@firebase/firestore-exp

@firebase/firestore-lite

Test Logs

Feiyang1 left a 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

schmidt-sebastian left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schmidt-sebastian Aug 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schmidt-sebastian commented Aug 20, 2020

schmidt-sebastian commented Sep 16, 2020

schmidt-sebastian commented Aug 18, 2020 •

edited

Loading

changeset-bot bot commented Aug 18, 2020 •

edited

Loading

schmidt-sebastian Aug 19, 2020 •

edited

Loading

google-oss-bot commented Aug 18, 2020 •

edited

Loading

`@firebase/firestore-exp`

`@firebase/firestore-lite`

schmidt-sebastian Aug 19, 2020 •

edited

Loading