Merge pull request #1011 from forcedotcom/sm/structured-clone

refactor: use native structured-clone
forcedotcom · Jan 10, 2024 · 91bb531 · 91bb531
2 parents bd16906 + ae99dff
commit 91bb531
Show file tree

Hide file tree

Showing 9 changed files with 119 additions and 43 deletions.
diff --git a/src/config/configStore.ts b/src/config/configStore.ts
@@ -5,7 +5,7 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 
-import { AsyncOptionalCreatable, cloneJson } from '@salesforce/kit';
+import { AsyncOptionalCreatable } from '@salesforce/kit';
 import { entriesOf, isPlainObject } from '@salesforce/ts-types';
 import { definiteEntriesOf, definiteValuesOf, isJsonMap, isString, JsonMap, Optional } from '@salesforce/ts-types';
 import { Crypto } from '../crypto/crypto';
@@ -95,7 +95,7 @@ export abstract class BaseConfigStore<
 
     if (this.hasEncryption() && decrypt) {
       if (isJsonMap(rawValue)) {
-        return this.recursiveDecrypt(cloneJson(rawValue), key);
+        return this.recursiveDecrypt(structuredClone(rawValue), key);
       } else if (this.isCryptoKey(key)) {
         return this.decrypt(rawValue) as P[K] | ConfigValue;
       }
@@ -219,7 +219,7 @@ export abstract class BaseConfigStore<
    */
   public getContents(decrypt = false): Readonly<P> {
     if (this.hasEncryption() && decrypt) {
-      return this.recursiveDecrypt(cloneJson(this.contents?.value ?? {})) as P;
+      return this.recursiveDecrypt(structuredClone(this.contents?.value ?? {})) as P;
     }
     return this.contents?.value ?? ({} as P);
   }

diff --git a/src/global.ts b/src/global.ts
@@ -83,7 +83,10 @@ export class Global {
    * ```
    */
   public static getEnvironmentMode(): Mode {
-    return Mode[env.getKeyOf('SFDX_ENV', Mode, Mode.PRODUCTION, (value) => value.toUpperCase())];
+    const envValue = env.getString('SF_ENV') ?? env.getString('SFDX_ENV', Mode.PRODUCTION);
+    return envValue in Mode || envValue.toUpperCase() in Mode
+      ? Mode[envValue.toUpperCase() as keyof typeof Mode]
+      : Mode.PRODUCTION;
   }
 
   /**

diff --git a/src/org/authInfo.ts b/src/org/authInfo.ts
@@ -11,7 +11,7 @@ import { resolve as pathResolve } from 'node:path';
 import * as os from 'node:os';
 import * as fs from 'node:fs';
 import { Record as RecordType } from 'jsforce';
-import { AsyncOptionalCreatable, cloneJson, env, isEmpty, parseJson, parseJsonMap } from '@salesforce/kit';
+import { AsyncOptionalCreatable, env, isEmpty, parseJson, parseJsonMap } from '@salesforce/kit';
 import {
   AnyJson,
   asString,
@@ -847,7 +847,7 @@ export class AuthInfo extends AsyncOptionalCreatable<AuthInfo.Options> {
     let authConfig: AuthFields;
 
     if (options) {
-      options = cloneJson(options);
+      options = structuredClone(options);
 
       if (this.isTokenOptions(options)) {
         authConfig = options;

diff --git a/src/schema/validator.ts b/src/schema/validator.ts
@@ -110,7 +110,7 @@ export class SchemaValidator {
 
     // AJV will modify the original json object. We need to make a clone of the
     // json to keep this backwards compatible with JSEN functionality
-    const jsonClone: T = JSON.parse(JSON.stringify(json)) as T;
+    const jsonClone = structuredClone(json);
 
     const valid = validate(jsonClone);
 

diff --git a/test/unit/config/configTest.ts b/test/unit/config/configTest.ts
@@ -11,7 +11,7 @@
 
 import * as fs from 'node:fs';
 import { stubMethod } from '@salesforce/ts-sinon';
-import { ensureString, JsonMap } from '@salesforce/ts-types';
+import { ensureString } from '@salesforce/ts-types';
 import { expect } from 'chai';
 import * as lockfileLib from 'proper-lockfile';
 import { Config, ConfigPropertyMeta } from '../../../src/config/config';
@@ -23,8 +23,6 @@ import { shouldThrowSync, TestContext } from '../../../src/testSetup';
 const configFileContentsString = '{"target-dev-hub": "configTest_devhub","target-org": "configTest_default"}';
 const configFileContentsJson = { 'target-dev-hub': 'configTest_devhub', 'target-org': 'configTest_default' };
 
-const clone = (obj: JsonMap) => JSON.parse(JSON.stringify(obj));
-
 describe('Config', () => {
   const $$ = new TestContext();
 
@@ -99,7 +97,7 @@ describe('Config', () => {
     it('calls Config.write with updated file contents', async () => {
       const writeStub = stubMethod($$.SANDBOX, fs.promises, 'writeFile');
 
-      const expectedFileContents = clone(configFileContentsJson);
+      const expectedFileContents = structuredClone(configFileContentsJson);
       const newUsername = 'updated_val';
       expectedFileContents['target-org'] = newUsername;
 
@@ -109,7 +107,7 @@ describe('Config', () => {
     });
 
     it('calls Config.write with deleted file contents', async () => {
-      const expectedFileContents = clone(configFileContentsJson);
+      const expectedFileContents = structuredClone(configFileContentsJson);
       const newUsername = 'updated_val';
       expectedFileContents['target-org'] = newUsername;
 
@@ -236,8 +234,8 @@ describe('Config', () => {
       stubMethod($$.SANDBOX, fs.promises, 'stat').resolves({ mtimeNs: BigInt(new Date().valueOf() - 1_000 * 60 * 5) });
       const writeStub = stubMethod($$.SANDBOX, fs.promises, 'writeFile');
 
-      const expectedFileContents = clone(configFileContentsJson);
-      delete expectedFileContents['target-org'];
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+      const { 'target-org': deleteThis, ...expectedFileContents } = structuredClone(configFileContentsJson);
 
       const config = await Config.create({ isGlobal: false });
       config.unset('target-org');

diff --git a/test/unit/globalTest.ts b/test/unit/globalTest.ts
@@ -5,26 +5,93 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 import { expect } from 'chai';
+import { isString } from '@salesforce/ts-types';
 import { Global, Mode } from '../../src/global';
 
 describe('Global', () => {
   describe('environmentMode', () => {
-    const sfdxEnv = process.env.SFDX_ENV;
+    const originalEnv = { SFDX_ENV: process.env.SFDX_ENV, SF_ENV: process.env.SF_ENV };
+    const cleanEnv = () => Object.keys(originalEnv).map((key) => delete process.env[key]);
+
+    beforeEach(() => {
+      cleanEnv();
+    });
 
     after(() => {
-      process.env.SFDX_ENV = sfdxEnv;
+      cleanEnv();
+      Object.entries(originalEnv)
+        .filter(([, value]) => isString(value))
+        .map(([key, value]) => {
+          process.env[key] = value;
+        });
     });
 
-    it('uses SFDX_ENV mode', () => {
+    it('uses SFDX_ENV mode alone', () => {
       process.env.SFDX_ENV = 'development';
       expect(Global.getEnvironmentMode() === Mode.DEVELOPMENT).to.be.true;
       expect(Global.getEnvironmentMode() === Mode.PRODUCTION).to.be.false;
       expect(Global.getEnvironmentMode() === Mode.DEMO).to.be.false;
       expect(Global.getEnvironmentMode() === Mode.TEST).to.be.false;
     });
 
+    it('uses SF_ENV mode alone', () => {
+      process.env.SF_ENV = 'development';
+      expect(Global.getEnvironmentMode() === Mode.DEVELOPMENT).to.be.true;
+      expect(Global.getEnvironmentMode() === Mode.PRODUCTION).to.be.false;
+      expect(Global.getEnvironmentMode() === Mode.DEMO).to.be.false;
+      expect(Global.getEnvironmentMode() === Mode.TEST).to.be.false;
+    });
+
+    it('prefers SF_ENV mode', () => {
+      process.env.SF_ENV = 'test';
+      process.env.SFDX_ENV = 'development';
+      expect(Global.getEnvironmentMode() === Mode.DEVELOPMENT).to.be.false;
+      expect(Global.getEnvironmentMode() === Mode.PRODUCTION).to.be.false;
+      expect(Global.getEnvironmentMode() === Mode.DEMO).to.be.false;
+      expect(Global.getEnvironmentMode() === Mode.TEST).to.be.true;
+    });
+
+    it('finds uppercase', () => {
+      process.env.SF_ENV = 'TEST';
+      expect(Global.getEnvironmentMode() === Mode.DEVELOPMENT).to.be.false;
+      expect(Global.getEnvironmentMode() === Mode.PRODUCTION).to.be.false;
+      expect(Global.getEnvironmentMode() === Mode.DEMO).to.be.false;
+      expect(Global.getEnvironmentMode() === Mode.TEST).to.be.true;
+    });
+
+    it('finds lowercase', () => {
+      process.env.SF_ENV = 'demo';
+      expect(Global.getEnvironmentMode() === Mode.DEVELOPMENT).to.be.false;
+      expect(Global.getEnvironmentMode() === Mode.PRODUCTION).to.be.false;
+      expect(Global.getEnvironmentMode() === Mode.DEMO).to.be.true;
+      expect(Global.getEnvironmentMode() === Mode.TEST).to.be.false;
+    });
+
+    it('finds mixed case', () => {
+      process.env.SF_ENV = 'dEvelOpment';
+      expect(Global.getEnvironmentMode() === Mode.DEVELOPMENT).to.be.true;
+      expect(Global.getEnvironmentMode() === Mode.PRODUCTION).to.be.false;
+      expect(Global.getEnvironmentMode() === Mode.DEMO).to.be.false;
+      expect(Global.getEnvironmentMode() === Mode.TEST).to.be.false;
+    });
+
     it('is production by default', () => {
-      delete process.env.SFDX_ENV;
+      expect(Global.getEnvironmentMode() === Mode.DEVELOPMENT).to.be.false;
+      expect(Global.getEnvironmentMode() === Mode.PRODUCTION).to.be.true;
+      expect(Global.getEnvironmentMode() === Mode.DEMO).to.be.false;
+      expect(Global.getEnvironmentMode() === Mode.TEST).to.be.false;
+    });
+
+    it('defaults to production when invalid values are specified (SFDX)', () => {
+      process.env.SFDX_ENV = 'notARealMode';
+      expect(Global.getEnvironmentMode() === Mode.DEVELOPMENT).to.be.false;
+      expect(Global.getEnvironmentMode() === Mode.PRODUCTION).to.be.true;
+      expect(Global.getEnvironmentMode() === Mode.DEMO).to.be.false;
+      expect(Global.getEnvironmentMode() === Mode.TEST).to.be.false;
+    });
+
+    it('defaults to production when invalid values are specified (SF)', () => {
+      process.env.SF_ENV = 'notARealMode';
       expect(Global.getEnvironmentMode() === Mode.DEVELOPMENT).to.be.false;
       expect(Global.getEnvironmentMode() === Mode.PRODUCTION).to.be.true;
       expect(Global.getEnvironmentMode() === Mode.DEMO).to.be.false;

diff --git a/test/unit/loggerTest.ts b/test/unit/loggerTest.ts
@@ -9,6 +9,7 @@
 /* eslint-disable @typescript-eslint/no-unsafe-return */
 
 import { expect, config as chaiConfig } from 'chai';
+import { isString } from '@salesforce/ts-types';
 import { Logger, LoggerLevel, computeLevel } from '../../src/logger/logger';
 import { shouldThrowSync, TestContext } from '../../src/testSetup';
 
@@ -18,12 +19,20 @@ import { shouldThrowSync, TestContext } from '../../src/testSetup';
 chaiConfig.truncateThreshold = 0;
 describe('Logger', () => {
   const $$ = new TestContext();
-  const sfdxEnv = process.env.SFDX_ENV;
-  const logRotationPeriodBackup = process.env.SF_LOG_ROTATION_PERIOD;
-  const logRotationCountBackup = process.env.SF_LOG_ROTATION_COUNT;
-
-  beforeEach(async () => {
+  const originalEnv = {
+    SFDX_ENV: process.env.SFDX_ENV,
+    SF_ENV: process.env.SF_ENV,
+    SF_LOG_LEVEL: process.env.SF_LOG_LEVEL,
+    SF_DISABLE_LOG_FILE: process.env.SF_DISABLE_LOG_FILE,
+    SF_LOG_ROTATION_PERIOD: process.env.SF_LOG_ROTATION_PERIOD,
+    SF_LOG_ROTATION_COUNT: process.env.SF_LOG_ROTATION_COUNT,
+  };
+
+  const cleanEnv = () => Object.keys(originalEnv).map((key) => delete process.env[key]);
+  beforeEach(() => {
+    cleanEnv();
     process.env.SFDX_ENV = 'test';
+    process.env.SF_ENV = 'test';
 
     // Must restore the globally stubbed Logger.child method here.  Stubbed in testSetup.
     // @ts-expect-error: called is a sinon spy property
@@ -32,9 +41,15 @@ describe('Logger', () => {
 
   afterEach(() => {
     Logger.destroyRoot();
-    if (sfdxEnv) process.env.SFDX_ENV = sfdxEnv;
-    if (logRotationPeriodBackup) process.env.SF_LOG_ROTATION_PERIOD = logRotationPeriodBackup;
-    if (logRotationCountBackup) process.env.SF_LOG_ROTATION_COUNT = logRotationCountBackup;
+  });
+
+  after(() => {
+    cleanEnv();
+    Object.entries(originalEnv)
+      .filter(([, value]) => isString(value))
+      .map(([key, value]) => {
+        process.env[key] = value;
+      });
   });
 
   describe('constructor', () => {
@@ -47,13 +62,10 @@ describe('Logger', () => {
     });
 
     describe('DISABLE_LOG_FILE', () => {
-      const LOG_FILES_DISABLED = process.env.SF_DISABLE_LOG_FILE;
       before(() => {
         process.env.SF_DISABLE_LOG_FILE = 'true';
       });
-      after(() => {
-        process.env.SF_DISABLE_LOG_FILE = LOG_FILES_DISABLED;
-      });
+
       it('should construct a new named logger', async () => {
         const logger1 = new Logger({ name: 'testLogger-noop' });
         expect(logger1).to.be.instanceof(Logger);
@@ -66,9 +78,6 @@ describe('Logger', () => {
 
   describe('levels', () => {
     describe('level computation', () => {
-      afterEach(() => {
-        delete process.env.SF_LOG_LEVEL;
-      });
       it('should use a matching a level name when passed in', () => {
         expect(computeLevel('error')).to.equal('error');
       });

diff --git a/test/unit/messagesTest.ts b/test/unit/messagesTest.ts
@@ -10,7 +10,6 @@
 import * as fs from 'node:fs';
 import * as path from 'node:path';
 import { EOL } from 'node:os';
-import { cloneJson } from '@salesforce/kit';
 import { assert, expect } from 'chai';
 import { SinonStub } from 'sinon';
 import { Messages } from '../../src/messages';
@@ -30,8 +29,8 @@ describe('Messages', () => {
   msgMap.set('msg1', testMessages.msg1);
   msgMap.set('msg1.actions', testMessages.msg2);
   msgMap.set('msg2', testMessages.msg2);
-  msgMap.set('msg3', cloneJson(testMessages));
-  msgMap.get('msg3').msg3 = cloneJson(testMessages);
+  msgMap.set('msg3', structuredClone(testMessages));
+  msgMap.get('msg3').msg3 = structuredClone(testMessages);
   msgMap.set('manyMsgs', testMessages.manyMsgs);
 
   describe('getMessage', () => {

diff --git a/test/unit/org/authInfoTest.ts b/test/unit/org/authInfoTest.ts
@@ -12,7 +12,7 @@
 import * as pathImport from 'node:path';
 import * as dns from 'node:dns';
 import * as jwt from 'jsonwebtoken';
-import { cloneJson, env, includes } from '@salesforce/kit';
+import { env, includes } from '@salesforce/kit';
 import { spyMethod, stubMethod } from '@salesforce/ts-sinon';
 import { AnyJson, getJsonMap, JsonMap, toJsonMap } from '@salesforce/ts-types';
 import { expect } from 'chai';
@@ -335,7 +335,7 @@ describe('AuthInfo', () => {
           loginUrl: testOrg.loginUrl,
           privateKey: testOrg.privateKey,
         };
-        const jwtConfigClone = cloneJson(jwtConfig);
+        const jwtConfigClone = structuredClone(jwtConfig);
         const authResponse = {
           access_token: testOrg.accessToken,
           instance_url: testOrg.instanceUrl,
@@ -469,7 +469,7 @@ describe('AuthInfo', () => {
           loginUrl: testOrg.loginUrl,
           privateKey: testOrg.privateKey,
         };
-        const jwtConfigClone = cloneJson(jwtConfig);
+        const jwtConfigClone = structuredClone(jwtConfig);
         const authResponse = {
           access_token: testOrg.accessToken,
           instance_url: testOrg.instanceUrl,
@@ -535,7 +535,7 @@ describe('AuthInfo', () => {
           refreshToken: testOrg.refreshToken,
           loginUrl: testOrg.loginUrl,
         };
-        const refreshTokenConfigClone = cloneJson(refreshTokenConfig);
+        const refreshTokenConfigClone = structuredClone(refreshTokenConfig);
         const authResponse = {
           access_token: testOrg.accessToken,
           instance_url: testOrg.instanceUrl,
@@ -590,7 +590,7 @@ describe('AuthInfo', () => {
           loginUrl: testOrg.loginUrl,
           username: testOrg.username,
         };
-        const refreshTokenConfigClone = cloneJson(refreshTokenConfig);
+        const refreshTokenConfigClone = structuredClone(refreshTokenConfig);
         const authResponse = {
           access_token: testOrg.accessToken,
           instance_url: testOrg.instanceUrl,
@@ -643,7 +643,7 @@ describe('AuthInfo', () => {
           refreshToken: testOrg.refreshToken,
           loginUrl: testOrg.loginUrl,
         };
-        const refreshTokenConfigClone = cloneJson(refreshTokenConfig);
+        const refreshTokenConfigClone = structuredClone(refreshTokenConfig);
         const authResponse = {
           access_token: testOrg.accessToken,
           instance_url: testOrg.instanceUrl,
@@ -786,7 +786,7 @@ describe('AuthInfo', () => {
           authCode: testOrg.authcode,
           loginUrl: testOrg.loginUrl,
         };
-        const authCodeConfigClone = cloneJson(authCodeConfig);
+        const authCodeConfigClone = structuredClone(authCodeConfig);
         const authResponse = {
           access_token: testOrg.accessToken,
           instance_url: testOrg.instanceUrl,