Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: use native structured-clone #1011

Merged
merged 3 commits into from
Jan 10, 2024
Merged

refactor: use native structured-clone #1011

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c20bba8
refactor: use native structured-clone
mshanemc Dec 28, 2023
2934091
refactor: environment mode not using getKeyOf
mshanemc Dec 29, 2023
ae99dff
test: ut for env global env/mode changes
mshanemc Dec 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/config/configStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
Expand Down
5 changes: 4 additions & 1 deletion src/global.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/org/authInfo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/schema/validator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
12 changes: 5 additions & 7 deletions test/unit/config/configTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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();

Expand Down Expand Up @@ -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;

Expand All @@ -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;

Expand Down Expand Up @@ -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');
Expand Down
75 changes: 71 additions & 4 deletions test/unit/globalTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
39 changes: 24 additions & 15 deletions test/unit/loggerTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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
Expand All @@ -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', () => {
Expand All @@ -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);
Expand All @@ -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');
});
Expand Down
5 changes: 2 additions & 3 deletions test/unit/messagesTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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', () => {
Expand Down
14 changes: 7 additions & 7 deletions test/unit/org/authInfoTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Loading