refactor!: Remove extend and Treat Provided Options as Mutable (#2204)

* refactor: Remove `extend` This is the prelimary, `src`-only commit. * fix: object merging * chore: remove log * chore: minor clean-up * chore: more clean-up * refactor: remove `extend` * perf!: treat provided options as mutable
googleapis · Jun 13, 2023 · fc29cb4 · fc29cb4
1 parent 959dcdb
commit fc29cb4
Show file tree

Hide file tree

Showing 14 changed files with 167 additions and 210 deletions.
diff --git a/package.json b/package.json
@@ -58,7 +58,6 @@
     "compressible": "^2.0.12",
     "duplexify": "^4.0.0",
     "ent": "^2.2.0",
-    "extend": "^3.0.2",
     "gaxios": "^5.0.0",
     "google-auth-library": "^8.0.1",
     "mime": "^3.0.0",
@@ -75,7 +74,6 @@
     "@types/async-retry": "^1.4.3",
     "@types/compressible": "^2.0.0",
     "@types/ent": "^2.2.1",
-    "@types/extend": "^3.0.0",
     "@types/mime": "^3.0.0",
     "@types/mime-types": "^2.1.0",
     "@types/mocha": "^9.1.1",

diff --git a/src/bucket.ts b/src/bucket.ts
@@ -28,7 +28,6 @@ import {
 } from './nodejs-common';
 import {paginator} from '@google-cloud/paginator';
 import {promisifyAll} from '@google-cloud/promisify';
-import * as extend from 'extend';
 import * as fs from 'fs';
 import * as http from 'http';
 import * as mime from 'mime-types';
@@ -3270,7 +3269,7 @@ class Bucket extends ServiceObject {
 
     // You aren't allowed to set both predefinedAcl & acl properties on a bucket
     // so acl must explicitly be nullified.
-    const metadata = extend({}, options.metadata, {acl: null});
+    const metadata = {...options.metadata, acl: null};
 
     this.setMetadata(metadata, query, err => {
       if (err) {
@@ -3400,7 +3399,7 @@ class Bucket extends ServiceObject {
     callback =
       typeof optionsOrCallback === 'function' ? optionsOrCallback : callback;
 
-    const req = extend(true, {public: true}, options);
+    const req = {public: true, ...options};
 
     this.acl
       .add({
@@ -3514,7 +3513,7 @@ class Bucket extends ServiceObject {
     callback?: BodyResponseCallback
   ): void | Promise<[ResponseBody, Metadata]> {
     if (this.userProject && (!reqOpts.qs || !reqOpts.qs.userProject)) {
-      reqOpts.qs = extend(reqOpts.qs, {userProject: this.userProject});
+      reqOpts.qs = {...reqOpts.qs, userProject: this.userProject};
     }
     return super.request(reqOpts, callback!);
   }
@@ -3893,7 +3892,7 @@ class Bucket extends ServiceObject {
       const methodConfig = this.methods[method];
       if (typeof methodConfig === 'object') {
         if (typeof methodConfig.reqOpts === 'object') {
-          extend(methodConfig.reqOpts.qs, {userProject});
+          Object.assign(methodConfig.reqOpts.qs, {userProject});
         } else {
           methodConfig.reqOpts = {
             qs: {userProject},

diff --git a/src/file.ts b/src/file.ts
@@ -27,7 +27,6 @@ import {promisifyAll} from '@google-cloud/promisify';
 
 import compressible = require('compressible');
 import * as crypto from 'crypto';
-import * as extend from 'extend';
 import * as fs from 'fs';
 import * as mime from 'mime';
 import * as resumableUpload from './resumable-upload';
@@ -387,10 +386,6 @@ export interface SetStorageClassOptions {
   preconditionOpts?: PreconditionOptions;
 }
 
-interface SetStorageClassRequest extends SetStorageClassOptions {
-  storageClass?: string;
-}
-
 export interface SetStorageClassCallback {
   (err?: Error | null, apiResponse?: Metadata): void;
 }
@@ -1158,10 +1153,9 @@ class File extends ServiceObject<File> {
     if (typeof optionsOrCallback === 'function') {
       callback = optionsOrCallback;
     } else if (optionsOrCallback) {
-      options = optionsOrCallback;
+      options = {...optionsOrCallback};
     }
 
-    options = extend(true, {}, options);
     callback = callback || util.noop;
 
     let destBucket: Bucket;
@@ -1848,7 +1842,7 @@ class File extends ServiceObject<File> {
    */
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   createWriteStream(options: CreateWriteStreamOptions = {}): Writable {
-    options = extend(true, {metadata: {}}, options);
+    options.metadata ??= {};
 
     if (options.contentType) {
       options.metadata.contentType = options.contentType;
@@ -3069,7 +3063,7 @@ class File extends ServiceObject<File> {
     // You aren't allowed to set both predefinedAcl & acl properties on a file,
     // so acl must explicitly be nullified, destroying all previous acls on the
     // file.
-    const metadata = extend({}, options.metadata, {acl: null});
+    const metadata = {...options.metadata, acl: null};
 
     this.setMetadata(metadata, query, callback!);
   }
@@ -3757,19 +3751,17 @@ class File extends ServiceObject<File> {
       typeof optionsOrCallback === 'function' ? optionsOrCallback : callback;
     const options =
       typeof optionsOrCallback === 'object' ? optionsOrCallback : {};
-    const req = extend<SetStorageClassRequest, SetStorageClassOptions>(
-      true,
-      {},
-      options
-    );
 
-    // In case we get input like `storageClass`, convert to `storage_class`.
-    req.storageClass = storageClass
-      .replace(/-/g, '_')
-      .replace(/([a-z])([A-Z])/g, (_, low, up) => {
-        return low + '_' + up;
-      })
-      .toUpperCase();
+    const req = {
+      ...options,
+      // In case we get input like `storageClass`, convert to `storage_class`.
+      storageClass: storageClass
+        .replace(/-/g, '_')
+        .replace(/([a-z])([A-Z])/g, (_, low, up) => {
+          return low + '_' + up;
+        })
+        .toUpperCase(),
+    };
 
     this.copy(this, req, (err, file, apiResponse) => {
       if (err) {
@@ -3813,20 +3805,14 @@ class File extends ServiceObject<File> {
    */
   startResumableUpload_(
     dup: Duplexify,
-    options: CreateResumableUploadOptions
+    options: CreateResumableUploadOptions = {}
   ): void {
-    options = extend(
-      true,
-      {
-        metadata: {},
-      },
-      options
-    );
+    options.metadata ??= {};
 
     const retryOptions = this.storage.retryOptions;
     if (
       !this.shouldRetryBasedOnPreconditionAndIdempotencyStrat(
-        options?.preconditionOpts
+        options.preconditionOpts
       )
     ) {
       retryOptions.autoRetry = false;
@@ -3884,14 +3870,11 @@ class File extends ServiceObject<File> {
    *
    * @private
    */
-  startSimpleUpload_(dup: Duplexify, options?: CreateWriteStreamOptions): void {
-    options = extend(
-      true,
-      {
-        metadata: {},
-      },
-      options
-    );
+  startSimpleUpload_(
+    dup: Duplexify,
+    options: CreateWriteStreamOptions = {}
+  ): void {
+    options.metadata ??= {};
 
     const apiEndpoint = this.storage.apiEndpoint;
     const bucketName = this.bucket.name;

diff --git a/src/nodejs-common/service-object.ts b/src/nodejs-common/service-object.ts
@@ -15,7 +15,6 @@
  */
 import {promisifyAll} from '@google-cloud/promisify';
 import {EventEmitter} from 'events';
-import * as extend from 'extend';
 import * as r from 'teeny-request';
 
 import {StreamRequestOptions} from '.';
@@ -290,17 +289,15 @@ class ServiceObject<T = any> extends EventEmitter {
     const methodConfig =
       (typeof this.methods.delete === 'object' && this.methods.delete) || {};
 
-    const reqOpts = extend(
-      true,
-      {
-        method: 'DELETE',
-        uri: '',
+    const reqOpts = {
+      method: 'DELETE',
+      uri: '',
+      ...methodConfig.reqOpts,
+      qs: {
+        ...methodConfig.reqOpts?.qs,
+        ...options,
       },
-      methodConfig.reqOpts,
-      {
-        qs: options,
-      }
-    );
+    };
 
     // The `request` method may have been overridden to hold any special
     // behavior. Ensure we call the original `request` method.
@@ -440,16 +437,14 @@ class ServiceObject<T = any> extends EventEmitter {
       (typeof this.methods.getMetadata === 'object' &&
         this.methods.getMetadata) ||
       {};
-    const reqOpts = extend(
-      true,
-      {
-        uri: '',
+    const reqOpts = {
+      uri: '',
+      ...methodConfig.reqOpts,
+      qs: {
+        ...methodConfig.reqOpts?.qs,
+        ...options,
       },
-      methodConfig.reqOpts,
-      {
-        qs: options,
-      }
-    );
+    };
 
     // The `request` method may have been overridden to hold any special
     // behavior. Ensure we call the original `request` method.
@@ -507,19 +502,19 @@ class ServiceObject<T = any> extends EventEmitter {
         this.methods.setMetadata) ||
       {};
 
-    const reqOpts = extend(
-      true,
-      {},
-      {
-        method: 'PATCH',
-        uri: '',
+    const reqOpts = {
+      method: 'PATCH',
+      uri: '',
+      ...methodConfig.reqOpts,
+      json: {
+        ...methodConfig.reqOpts?.json,
+        ...metadata,
       },
-      methodConfig.reqOpts,
-      {
-        json: metadata,
-        qs: options,
-      }
-    );
+      qs: {
+        ...methodConfig.reqOpts?.qs,
+        ...options,
+      },
+    };
 
     // The `request` method may have been overridden to hold any special
     // behavior. Ensure we call the original `request` method.
@@ -551,7 +546,7 @@ class ServiceObject<T = any> extends EventEmitter {
     reqOpts: DecorateRequestOptions | StreamRequestOptions,
     callback?: BodyResponseCallback
   ): void | r.Request {
-    reqOpts = extend(true, {}, reqOpts);
+    reqOpts = {...reqOpts};
 
     if (this.projectId) {
       reqOpts.projectId = this.projectId;
@@ -612,7 +607,7 @@ class ServiceObject<T = any> extends EventEmitter {
    * @param {string} reqOpts.uri - A URI relative to the baseUrl.
    */
   requestStream(reqOpts: DecorateRequestOptions): r.Request {
-    const opts = extend(true, reqOpts, {shouldReturnStream: true});
+    const opts = {...reqOpts, shouldReturnStream: true};
     return this.request_(opts as StreamRequestOptions);
   }
 }

diff --git a/src/nodejs-common/service.ts b/src/nodejs-common/service.ts
@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import * as extend from 'extend';
 import {AuthClient, GoogleAuth, GoogleAuthOptions} from 'google-auth-library';
 import * as r from 'teeny-request';
 import * as uuid from 'uuid';
@@ -111,15 +110,16 @@ export class Service {
     this.projectIdRequired = config.projectIdRequired !== false;
     this.providedUserAgent = options.userAgent;
 
-    const reqCfg = extend({}, config, {
+    const reqCfg = {
+      ...config,
       projectIdRequired: this.projectIdRequired,
       projectId: this.projectId,
-      authClient: options.authClient,
+      authClient: options.authClient || config.authClient,
       credentials: options.credentials,
       keyFile: options.keyFilename,
       email: options.email,
       token: options.token,
-    });
+    };
 
     this.makeAuthenticatedRequest =
       util.makeAuthenticatedRequestFactory(reqCfg);
@@ -192,7 +192,7 @@ export class Service {
     reqOpts: DecorateRequestOptions | StreamRequestOptions,
     callback?: BodyResponseCallback
   ): void | r.Request {
-    reqOpts = extend(true, {}, reqOpts, {timeout: this.timeout});
+    reqOpts = {...reqOpts, timeout: this.timeout};
     const isAbsoluteUrl = reqOpts.uri.indexOf('http') === 0;
     const uriComponents = [this.baseUrl];
 
@@ -244,12 +244,13 @@ export class Service {
     if (this.providedUserAgent) {
       userAgent = `${this.providedUserAgent} ${userAgent}`;
     }
-    reqOpts.headers = extend({}, reqOpts.headers, {
+    reqOpts.headers = {
+      ...reqOpts.headers,
       'User-Agent': userAgent,
       'x-goog-api-client': `gl-node/${process.versions.node} gccl/${
         pkg.version
       } gccl-invocation-id/${uuid.v4()}`,
-    });
+    };
 
     if (reqOpts.shouldReturnStream) {
       return this.makeAuthenticatedRequest(reqOpts) as {} as r.Request;
@@ -279,7 +280,7 @@ export class Service {
    * @param {string} reqOpts.uri - A URI relative to the baseUrl.
    */
   requestStream(reqOpts: DecorateRequestOptions): r.Request {
-    const opts = extend(true, reqOpts, {shouldReturnStream: true});
+    const opts = {...reqOpts, shouldReturnStream: true};
     return (Service.prototype.request_ as Function).call(this, opts);
   }
 }