feat!: v3 is now the default API surface #355

bcoe · 2019-10-25T19:47:30Z

BREAKING CHANGE: this significantly changes TypeScript types and API surface from the v2 API. Reference samples/ for help making the migration from v2 to v3.

BREAKING CHANGE: this signficantly changes TypeScript types and API surface from the v2 API. Reference samples for help making the migration from v2 to v3.

bcoe

In this PR we move from v2 (a hand written TypeScript veneer) to an auto-generated v3 as the default export.

Whereas v3beta1 did not have any TypeScript typings, v3 uses the new TypeScript micro-generator and does have types.

bcoe · 2019-10-25T19:50:10Z

tslint.json

@@ -1,3 +1,6 @@
 {
-  "extends": "gts/tslint.json"
+  "extends": "gts/tslint.json",
+  "rules": {


these new rules were added during @alexander-fenster's generation of the client, and I believe are automatic.

We'll have two instances of @ts-ignore for now (in the place where we load protos). They might go away later if I figure out how to deal with that. Those are only for dealing with dynamic protos.

bcoe · 2019-10-25T19:50:14Z

tsconfig.json

    "rootDir": ".",
-    "outDir": "build"


these new rules were added during @alexander-fenster's generation of the client, and I believe are automatic.

bcoe · 2019-10-25T19:51:55Z

system-test/translate.ts

+      for (const input of INPUT) {
+        const [result] = await translate.detectLanguage({
+          content: input.content,
+          parent: `projects/${projectId}`,


It seems like a slightly more difficult to work with API surface, that we now need to provide a parent. I noticed this was also the case for v3beta1 @nnegrey?

Translate now has user managed resources and locations

👍 so this is an expected change to the API? thought it might be a bug, and was talking to @alexander-fenster about it, sounds like it's not.

The only difference with JS gapic-generator is that gapic-generator provides a helper function for this:

const contents = []; const targetLanguageCode = ''; const formattedParent = client.locationPath('[PROJECT]', '[LOCATION]'); // ^^^ we don't have this yet in TypeScript so // we need to concatenate a string manually const request = { contents: contents, targetLanguageCode: targetLanguageCode, parent: formattedParent, }; const [response] = await client.translateText(request);

We will provide the same interface soon (@xiaozhenliu-gg5 is working on it). But the fact that it now requires project to be passed is an API change, not much we can do here.

bcoe · 2019-10-25T19:52:28Z

samples/translate.js

-  });
-
+  const projectId = await translate.getProjectId();
+  for (const text of texts) {


in the generated API, detect becomes detectLanguage.

codecov · 2019-10-25T19:59:27Z

Codecov Report

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

@@          Coverage Diff          @@
##           master   #355   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines           3     85   +82     
=====================================
+ Hits            3     85   +82

Impacted Files	Coverage Δ
src/index.ts	`100% <100%> (ø)`	⬆️

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 625e3f4...cc7fcf0. Read the comment docs.

nnegrey

v2 samples need to remain as is for docs. New v3 samples will be added.

nnegrey · 2019-10-25T20:09:02Z

v2 samples should just change to import the v2 client library

bcoe · 2019-10-25T20:14:03Z

@nnegrey this seems weird if we're making the default export of our library v3 we should have default examples that display the usage of the library if someone imports the default export:

const {TranslationServiceClient} = require('@google-cloud/translate');

I'd potentially suggest creating a v2 folder that shows the v2 usage, but strongly advise that; If we want v3 to be the default export of this library (which is what has been suggested by the product team) we should move the canonical examples over to the default export ... otherwise we're basically saying. "This library is totally v3, but we like the v2 API surface better so here are the samples from it.".

JustinBeckwith · 2019-10-25T20:16:25Z

.bots/header-checker-lint.json

@@ -0,0 +1,3 @@
+{
+  "ignoreFiles": ["protos/*"]


Are we sure about that? Instead, I'd rather we make the change in the underlying generator/protos if needed. If we don't want to block on that, please leave a TODO with a reference to a tracking bug so we don't lose this.

@JustinBeckwith created a TODO on this repo, #356

@alexander-fenster could you perhaps create a corresponding tracking issue, for adding license headers, and reference #356.

JustinBeckwith · 2019-10-25T20:18:51Z

tslint.json

-  "extends": "gts/tslint.json"
+  "extends": "gts/tslint.json",
+  "rules": {
+    "ban-ts-ignore": false


I would strongly advise against that flag. @alexander-fenster what's up here?

bcoe · 2019-10-25T20:21:54Z

@nnegrey and I had an out of band conversation, I'm comfortable with us sticking with default examples being v2 for now; but I've advised that it feels a little confusing, and we'll gradually update them over time (not a release blocker).

bcoe · 2019-10-28T16:30:01Z

samples/v3/translate_batch_translate_text_with_glossary_and_model.js

+  async function batchTranslateTextWithGlossaryAndModel() {
+    // Construct request
+    const request = {
+      parent: `projects/${projectId}/locations/${location}`,


this will be fixed in an upcoming "feature" release, in which we re-introduce the helper for parent:

see: https://github.com/googleapis/nodejs-translate/issues/359

nnegrey

LGTM (but noting that I wrote the samples)

bcoe · 2019-10-28T19:34:40Z

samples/quickstart.js


  // Instantiates a client
  const translate = new Translate({projectId});

-  // The text to translate
-  const text = 'Hello, world!';
+  async function quickStart() {


thanks for doing this 👍 this makes the embedded samples much better.

* Updates [google-cloud/translate][1] dependency to version `5.0.0`. * Updates declaration of clients to specify the version 2 of the API instead of the version 3, which is the default in the Translate library. Note: The google-cloud/translate library made v3 the default in googleapis/nodejs-translate#355 [1]: https://github.com/googleapis/nodejs-translate

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 25, 2019

alexander-fenster and others added 4 commits October 25, 2019 12:47

feat: adding v3 module in TypeScript

5e96613

tsignore

bea6364

gts fix

310cf60

feat!: v3 is now the default API surface

1b3400c

BREAKING CHANGE: this signficantly changes TypeScript types and API surface from the v2 API. Reference samples for help making the migration from v2 to v3.

bcoe force-pushed the translate-v3-typescript branch from fb24b8f to 1b3400c Compare October 25, 2019 19:49

bcoe commented Oct 25, 2019

View reviewed changes

chore: fix up license headers

3420b75

nnegrey suggested changes Oct 25, 2019

View reviewed changes

JustinBeckwith reviewed Oct 25, 2019

View reviewed changes

bcoe mentioned this pull request Oct 25, 2019

remove license header ignore rules #356

Closed

nnegrey and others added 6 commits October 25, 2019 15:30

Add Translation v3 samples

2b69137

Fix License headers

7129b30

Test License headers

59f8077

Linter

a0e9592

Update quickstart.js

5c144d3

chore: slight formatting fix

bbe3ae1

bcoe commented Oct 28, 2019

View reviewed changes

nnegrey approved these changes Oct 28, 2019

View reviewed changes

nnegrey added 2 commits October 28, 2019 12:14

Update v2 samples to use async functions inside region tag code

d9a1363

Remove unncessary async on outer functions

3f06c9d

bcoe added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 28, 2019

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 28, 2019

chore: address linting issue

cc7fcf0

bcoe commented Oct 28, 2019

View reviewed changes

bcoe merged commit 91169b4 into master Oct 28, 2019

bcoe deleted the translate-v3-typescript branch October 28, 2019 20:33

ricalo mentioned this pull request Oct 30, 2019

Updates speech-to-speech and ocr-app samples. GoogleCloudPlatform/nodejs-docs-samples#1532

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: v3 is now the default API surface #355

feat!: v3 is now the default API surface #355

bcoe commented Oct 25, 2019

bcoe left a comment

bcoe Oct 25, 2019

alexander-fenster Oct 25, 2019

bcoe Oct 25, 2019

bcoe Oct 25, 2019

nnegrey Oct 25, 2019

bcoe Oct 25, 2019

alexander-fenster Oct 25, 2019

bcoe Oct 25, 2019

codecov bot commented Oct 25, 2019 •

edited

Loading

nnegrey left a comment

nnegrey commented Oct 25, 2019

bcoe commented Oct 25, 2019

JustinBeckwith Oct 25, 2019

bcoe Oct 25, 2019

JustinBeckwith Oct 25, 2019

bcoe commented Oct 25, 2019

bcoe Oct 28, 2019

nnegrey left a comment

bcoe Oct 28, 2019

feat!: v3 is now the default API surface #355

feat!: v3 is now the default API surface #355

Conversation

bcoe commented Oct 25, 2019

bcoe 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

Choose a reason for hiding this comment

codecov bot commented Oct 25, 2019 • edited Loading

Codecov Report

nnegrey left a comment

Choose a reason for hiding this comment

nnegrey commented Oct 25, 2019

bcoe commented Oct 25, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcoe commented Oct 25, 2019

Choose a reason for hiding this comment

nnegrey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 25, 2019 •

edited

Loading