Adding Typescript support #232

schmidt-sebastian · 2018-07-02T19:42:59Z

Adding Typescript support to Firestore.

Note:

I only converted path.js to TS for now (as a proof of concept)
I did not (yet) run gts fix since it re-formats almost everything (which is why as of now all tests are failing)

The tests are also spewing errors about Google Default Credentials not working. I am not sure if this is new in this PR, but it doesn't (yet) cause problems. It might make sense to fix that in a follow up.

codecov · 2018-07-02T19:49:03Z

Codecov Report

Merging #232 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #232    +/-   ##
======================================
  Coverage     100%   100%            
======================================
  Files          13     12     -1     
  Lines        1846   1740   -106     
======================================
- Hits         1846   1740   -106

Impacted Files	Coverage Δ
src/index.js	`100% <100%> (ø)`	⬆️
src/write-batch.js	`100% <0%> (ø)`	⬆️
src/transaction.js	`100% <0%> (ø)`	⬆️
src/validate.js	`100% <0%> (ø)`	⬆️
src/document.js	`100% <0%> (ø)`	⬆️
src/watch.js	`100% <0%> (ø)`	⬆️
src/backoff.js	`100% <0%> (ø)`	⬆️
src/timestamp.js	`100% <0%> (ø)`	⬆️
src/reference.js	`100% <0%> (ø)`	⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 116c7e2...6cd1dc1. Read the comment docs.

JustinBeckwith · 2018-07-02T20:02:08Z

Wow, this is a big PR 🤣 Usually when we do a TypeScript transition, we're very careful to take a minimum number of steps. I have those pretty well laid out here:
googleapis/nodejs-translate#51

The issue with any library that uses GAPIC is that we're using generated JavaScript code. How are you getting around that out of curiosity?

schmidt-sebastian · 2018-07-02T20:27:08Z

Wow, this is a big PR 🤣 Usually when we do a TypeScript transition, we're very careful to take a minimum number of steps. I have those pretty well laid out here:
googleapis/nodejs-translate#51

The issue with any library that uses GAPIC is that we're using generated JavaScript code. How are you getting around that out of curiosity?

The PR was rather large since it contained a bunch of reformatting done by gts fix. I have reverted these changes for the first round of review.

I want to convert the client one file at a time and am leaving JS support on for now.

.nycrc

-    "src/*{/*,/**/*}.js",
-    "src/*/v*/*.js",
-    "test/**/*.js"
+    "build/src/*{/*,/**/*}.js",


package.json

@@ -8,14 +8,11 @@
    "node": ">=4.0.0"
  },
  "repository": "googleapis/nodejs-firestore",
-  "main": "./src/index.js",
+  "main": "./build/src/index.js",
+  "types": "./types/firestore.d.ts",


package.json

-    "test": "npm run cover"
+    "system-test": "mocha build/system-test/*.js --timeout 600000",
+    "conformance": "mocha build/conformance/*.js --no-timeouts",
+    "test-no-cover": "mocha build/test/*.js --no-timeouts",


src/path.ts

-      : Array.prototype.slice.call(arguments, 2);
-
+  constructor(
+      private readonly projectId: string, private readonly databaseId: string,


src/path.ts

-  construct(segments) {
-    return new ResourcePath(this._projectId, this._databaseId, segments);
+  construct(segments: string[]): ResourcePath {
+    return new ResourcePath(this.projectId, this.databaseId, ...segments);


src/path.ts

-      other._projectId !== '{{projectId}}'
-    ) {
-      if (this._projectId < other._projectId) {
+    if (this.projectId !== '{{projectId}}' &&


src/path.ts

-    segments = is.array(segments)
-      ? segments
-      : Array.prototype.slice.call(arguments);
+    segments = segments.length === 1 && is.array(segments[0]) ?


schmidt-sebastian

Addressed comments.

.nycrc

-    "src/*{/*,/**/*}.js",
-    "src/*/v*/*.js",
-    "test/**/*.js"
+    "build/src/*{/*,/**/*}.js",


package.json

@@ -8,14 +8,11 @@
    "node": ">=4.0.0"
  },
  "repository": "googleapis/nodejs-firestore",
-  "main": "./src/index.js",
+  "main": "./build/src/index.js",
+  "types": "./types/firestore.d.ts",


package.json

-    "test": "npm run cover"
+    "system-test": "mocha build/system-test/*.js --timeout 600000",
+    "conformance": "mocha build/conformance/*.js --no-timeouts",
+    "test-no-cover": "mocha build/test/*.js --no-timeouts",


src/path.ts

-  construct(segments) {
-    return new ResourcePath(this._projectId, this._databaseId, segments);
+  construct(segments: string[]): ResourcePath {
+    return new ResourcePath(this.projectId, this.databaseId, ...segments);


src/path.ts

-      other._projectId !== '{{projectId}}'
-    ) {
-      if (this._projectId < other._projectId) {
+    if (this.projectId !== '{{projectId}}' &&


src/path.ts

-    segments = is.array(segments)
-      ? segments
-      : Array.prototype.slice.call(arguments);
+    segments = segments.length === 1 && is.array(segments[0]) ?


tsconfig.json

+    "rootDir": ".",
+    "outDir": "build",
+    "target": "es6",
+    "noImplicitAny": false


tsconfig.json

-    "moduleResolution": "node",
-    "outDir": "dist/es6",
-    "rootDir": "./",
+    "declaration": false,


JustinBeckwith

LGTM w/ nits

.prettierrc

@@ -5,4 +5,4 @@ semi: true
 singleQuote: true
 tabWidth: 2
 trailingComma: es5
-useTabs: false
+useTabs: false


package.json

@@ -8,14 +8,11 @@
    "node": ">=4.0.0"
  },
  "repository": "googleapis/nodejs-firestore",
-  "main": "./src/index.js",
+  "main": "./build/src/index.js",
+  "types": "./types/firestore.d.ts",


googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 2, 2018

ghost assigned schmidt-sebastian Jul 2, 2018

schmidt-sebastian force-pushed the tstools branch from 58bbcc1 to 3e7812e Compare July 2, 2018 20:16

Adding Typescript tooling

1490a8f

schmidt-sebastian force-pushed the tstools branch from 3e7812e to 1490a8f Compare July 2, 2018 20:21

schmidt-sebastian requested a review from JustinBeckwith July 2, 2018 20:25

schmidt-sebastian assigned JustinBeckwith Jul 2, 2018

JustinBeckwith reviewed Jul 2, 2018

View reviewed changes

ofrobots reviewed Jul 2, 2018

View reviewed changes

src/path.ts Outdated

segments = is.array(segments)

? segments

: Array.prototype.slice.call(arguments);

segments = segments.length === 1 && is.array(segments[0]) ?

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Addressing comments

4a96b7e

schmidt-sebastian commented Jul 2, 2018

View reviewed changes

schmidt-sebastian added 6 commits July 2, 2018 16:44

Merge branch 'master' into tstools

bcb3733

Merge

1bdd7ff

Remove spread operator

007bad8

Merge

8e789c4

Add package lock

4e2b597

Fixing Eslint

c7f9eae

ofrobots reviewed Jul 3, 2018

View reviewed changes

JustinBeckwith approved these changes Jul 3, 2018

View reviewed changes

JSON comments

6cd1dc1

schmidt-sebastian merged commit d5573ff into master Jul 3, 2018

schmidt-sebastian deleted the tstools branch July 24, 2018 04:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Typescript support #232

Adding Typescript support #232

schmidt-sebastian commented Jul 2, 2018 •

edited

codecov bot commented Jul 2, 2018 •

edited

JustinBeckwith commented Jul 2, 2018

schmidt-sebastian commented Jul 2, 2018

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

schmidt-sebastian left a comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

JustinBeckwith left a comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Adding Typescript support #232

Adding Typescript support #232

Conversation

schmidt-sebastian commented Jul 2, 2018 • edited

codecov bot commented Jul 2, 2018 • edited

Codecov Report

JustinBeckwith commented Jul 2, 2018

schmidt-sebastian commented Jul 2, 2018

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

schmidt-sebastian left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

JustinBeckwith left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

schmidt-sebastian commented Jul 2, 2018 •

edited

codecov bot commented Jul 2, 2018 •

edited