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.

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: don't mutate params #1047

Merged
merged 3 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 6 additions & 0 deletions .sfdevrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@
"output": ["lib/**", "*.tsbuildinfo"],
"clean": "if-file-deleted"
},
"build": {
"dependencies": ["compile", "lint"]
},
"compile-typedoc": {
"command": "tsc -p typedocExamples"
},
"link-check": {
"command": "node -e \"process.exit(process.env.CI ? 0 : 1)\" || linkinator \"./*.md\" --skip \"examples/README.md|CHANGELOG.md|node_modules|test/|confluence.internal.salesforce.com|my.salesforce.com|%s\" --markdown --retry --directory-listing --verbosity error",
"files": ["./*.md", "./examples/**/*.md", "./messages/**/*.md", "./!(CHANGELOG).md"],
Expand Down
4,782 changes: 1,089 additions & 3,693 deletions CHANGELOG.md

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
"link-check": "wireit",
"lint": "wireit",
"lint-fix": "yarn sf-lint --fix",
"postcompile": "tsc -p typedocExamples",
"prepack": "sf-prepack",
"prepare": "sf-install",
"test": "wireit",
Expand Down Expand Up @@ -160,6 +159,9 @@
"./!(CHANGELOG).md"
],
"output": []
},
"compile-typedoc": {
"command": "tsc -p typedocExamples"
}
}
}
19 changes: 10 additions & 9 deletions src/config/configStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,18 +144,19 @@ export abstract class BaseConfigStore<
* @param value The value.
*/
public set<K extends Key<P>>(key: K, value: P[K]): void {
let resolvedValue = value;
if (this.hasEncryption()) {
if (isJsonMap(value)) {
value = this.recursiveEncrypt(value, key as string) as P[K];
if (isJsonMap(resolvedValue)) {
resolvedValue = this.recursiveEncrypt(resolvedValue, key as string) as P[K];
} else if (this.isCryptoKey(key as string)) {
value = this.encrypt(value) as P[K];
resolvedValue = this.encrypt(resolvedValue) as P[K];
}
}
// set(key, undefined) means unset
if (value === undefined) {
if (resolvedValue === undefined) {
this.unset(key);
} else {
this.contents.set(key, value);
this.contents.set(key, resolvedValue);
}
}

Expand Down Expand Up @@ -276,10 +277,8 @@ export abstract class BaseConfigStore<
* @param contents The contents.
*/
protected setContents(contents: P = {} as P): void {
if (this.hasEncryption()) {
contents = this.recursiveEncrypt(contents);
}
entriesOf(contents).map(([key, value]) => {
const maybeEncryptedContents = this.hasEncryption() ? this.recursiveEncrypt(contents) : contents;
entriesOf(maybeEncryptedContents).map(([key, value]) => {
this.contents.set(key, value);
});
}
Expand Down Expand Up @@ -420,6 +419,8 @@ export abstract class BaseConfigStore<
this.recursiveCrypto(method, [...keyPaths, key, newKey], value);
}
} else if (this.isCryptoKey(key)) {
// I think this side effect is intentional
// eslint-disable-next-line no-param-reassign
data[key] = method(value);
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/global.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,15 @@ export class Global {
* @param dirPath The directory path to be created within {@link Global.SFDX_DIR}.
*/
public static async createDir(dirPath?: string): Promise<void> {
dirPath = dirPath ? path.join(Global.SFDX_DIR, dirPath) : Global.SFDX_DIR;
const resolvedPath = dirPath ? path.join(Global.SFDX_DIR, dirPath) : Global.SFDX_DIR;
try {
if (process.platform === 'win32') {
await fs.promises.mkdir(dirPath, { recursive: true });
await fs.promises.mkdir(resolvedPath, { recursive: true });
} else {
await fs.promises.mkdir(dirPath, { recursive: true, mode: 0o700 });
await fs.promises.mkdir(resolvedPath, { recursive: true, mode: 0o700 });
}
} catch (error) {
throw new SfError(`Failed to create directory or set permissions for: ${dirPath}`);
throw new SfError(`Failed to create directory or set permissions for: ${resolvedPath}`);
}
}
}
25 changes: 12 additions & 13 deletions src/logger/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,11 +256,11 @@ export class Logger {
* @see {@Link LoggerLevel}
*/
public static getLevelByName(levelName: string): LoggerLevelValue {
levelName = levelName.toUpperCase();
if (!isKeyOf(LoggerLevel, levelName)) {
throw new SfError(`Invalid log level "${levelName}".`, 'UnrecognizedLoggerLevelNameError');
const upperLevel = levelName.toUpperCase();
if (!isKeyOf(LoggerLevel, upperLevel)) {
throw new SfError(`Invalid log level "${upperLevel}".`, 'UnrecognizedLoggerLevelNameError');
}
return LoggerLevel[levelName];
return LoggerLevel[upperLevel];
}

/** get the bare (pino) logger instead of using the class hierarchy */
Expand Down Expand Up @@ -309,11 +309,8 @@ export class Logger {
* ```
*/
public setLevel(level?: LoggerLevelValue): Logger {
if (level == null) {
const logLevelFromEnvVar = new Env().getString('SF_LOG_LEVEL');
level = logLevelFromEnvVar ? Logger.getLevelByName(logLevelFromEnvVar) : Logger.DEFAULT_LEVEL;
}
this.pinoLogger.level = this.pinoLogger.levels.labels[level] ?? this.pinoLogger.levels.labels[Logger.DEFAULT_LEVEL];
this.pinoLogger.level =
this.pinoLogger.levels.labels[level ?? getDefaultLevel()] ?? this.pinoLogger.levels.labels[Logger.DEFAULT_LEVEL];
return this;
}

Expand Down Expand Up @@ -342,10 +339,7 @@ export class Logger {
*/
public readLogContentsAsText(): string {
if (this.memoryLogger) {
return this.memoryLogger.loggedData.reduce((accum, line) => {
accum += JSON.stringify(line) + os.EOL;
return accum;
}, '');
return this.memoryLogger?.loggedData.map((line) => JSON.stringify(line)).join(os.EOL);
} else {
this.pinoLogger.warn(
'readLogContentsAsText is not supported for file streams, only used when useMemoryLogging is true'
Expand Down Expand Up @@ -511,3 +505,8 @@ const numberToLevel = (level: number): string =>
pino.levels.labels[level] ??
Object.entries(pino.levels.labels).find(([value]) => Number(value) > level)?.[1] ??
'warn';

const getDefaultLevel = (): LoggerLevel => {
const logLevelFromEnvVar = new Env().getString('SF_LOG_LEVEL');
return logLevelFromEnvVar ? Logger.getLevelByName(logLevelFromEnvVar) : Logger.DEFAULT_LEVEL;
};
39 changes: 22 additions & 17 deletions src/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import * as path from 'node:path';
import * as util from 'node:util';
import { fileURLToPath } from 'node:url';
import { AnyJson, asString, ensureJsonMap, ensureString, isJsonMap, isObject } from '@salesforce/ts-types';
import { ensureArray, NamedError, upperFirst } from '@salesforce/kit';
import { ensureArray, upperFirst } from '@salesforce/kit';
import { SfError } from './sfError';

export type Tokens = Array<string | boolean | number | null | undefined>;
Expand Down Expand Up @@ -362,17 +362,7 @@ export class Messages<T extends string> {
moduleMessagesDirPath = projectRoot;
}

if (!packageName) {
const message = `Invalid or missing package.json file at '${moduleMessagesDirPath}'. If not using a package.json, pass in a packageName.`;
try {
packageName = asString(ensureJsonMap(Messages.readFile(path.join(moduleMessagesDirPath, 'package.json'))).name);
if (!packageName) {
throw SfError.create({ message, name: 'MissingPackageName' });
}
} catch (err) {
throw SfError.create({ message, name: 'MissingPackageName', cause: err });
}
}
const resolvedPackageName = packageName ?? resolvePackageName(moduleMessagesDirPath);

moduleMessagesDirPath += `${path.sep}messages`;

Expand All @@ -385,7 +375,7 @@ export class Messages<T extends string> {
// When we support other locales, load them from /messages/<local>/<bundleName>.json
// Change generateFileLoaderFunction to handle loading locales.
} else if (stat.isFile()) {
this.importMessageFile(packageName, filePath);
this.importMessageFile(resolvedPackageName, filePath);
}
}
}
Expand Down Expand Up @@ -423,7 +413,7 @@ export class Messages<T extends string> {
}

// Don't use messages inside messages
throw new NamedError('MissingBundleError', `Missing bundle ${key} for locale ${Messages.getLocale()}.`);
throw new SfError(`Missing bundle ${key} for locale ${Messages.getLocale()}.`, 'MissingBundleError');
}

/**
Expand Down Expand Up @@ -591,9 +581,9 @@ export class Messages<T extends string> {
const msg = map.get(key);
if (!msg) {
// Don't use messages inside messages
throw new NamedError(
'MissingMessageError',
`Missing message ${this.bundleName}:${key} for locale ${Messages.getLocale()}.`
throw new SfError(
`Missing message ${this.bundleName}:${key} for locale ${Messages.getLocale()}.`,
'MissingMessageError'
);
}
const messages = ensureArray(msg);
Expand All @@ -603,3 +593,18 @@ export class Messages<T extends string> {
});
}
}

const resolvePackageName = (moduleMessagesDirPath: string): string => {
const errMessage = `Invalid or missing package.json file at '${moduleMessagesDirPath}'. If not using a package.json, pass in a packageName.`;
try {
const resolvedPackageName = asString(
ensureJsonMap(Messages.readFile(path.join(moduleMessagesDirPath, 'package.json'))).name
);
if (!resolvedPackageName) {
throw SfError.create({ message: errMessage, name: 'MissingPackageName' });
}
return resolvedPackageName;
} catch (err) {
throw SfError.create({ message: errMessage, name: 'MissingPackageName', cause: err });
}
};
Loading
Loading