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

feat: use oclif/core v4 #1614

Merged
merged 28 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
b09d551
feat: use core v4
mdonnalley Apr 11, 2024
5c7bf95
Merge branch 'main' into mdonnalley/core-v4
mdonnalley Apr 23, 2024
a80d695
feat: use sf logger for oclif
mdonnalley Apr 23, 2024
bbff4db
chore: clean up
mdonnalley Apr 23, 2024
d8e3de5
chore: use prereleases
mdonnalley Apr 24, 2024
5173421
chore: revert helpClass
mdonnalley Apr 24, 2024
371552b
feat: more performant command id dimming
mdonnalley May 2, 2024
4c1b9f8
Merge branch 'main' into mdonnalley/core-v4
mdonnalley May 3, 2024
37467a5
feat: use oclif/core top level exports
mdonnalley May 29, 2024
29074e3
Merge branch 'main' into mdonnalley/core-v4
mdonnalley May 29, 2024
c469b6f
chore: update deps
mdonnalley May 30, 2024
d3b8efa
chore: add back logger
mdonnalley May 30, 2024
5f6e3e5
Merge branch 'main' into mdonnalley/core-v4
mdonnalley May 30, 2024
1850fc7
chore: undo plugin wildcard
mdonnalley May 30, 2024
ff45487
chore: bump plugin-release-management
mdonnalley May 30, 2024
24d0778
fix: add json theme
mdonnalley May 31, 2024
ddce8c1
Merge branch 'main' into mdonnalley/core-v4
mdonnalley Jun 4, 2024
c9aaabd
chore: bump sf-plugins-core
mdonnalley Jun 4, 2024
82cc750
chore: bump sf-plugins-core
mdonnalley Jun 4, 2024
31330d3
chore: bump sf-plugins-core
mdonnalley Jun 5, 2024
3469f7c
Merge branch 'main' into mdonnalley/core-v4
mdonnalley Jun 5, 2024
40a5afd
chore: bump debug
mshanemc Jun 5, 2024
719d3dd
perf: defer largish imports into conditionals
mshanemc Jun 5, 2024
ac7f720
chore: remove unused public methods
mshanemc Jun 5, 2024
87bf106
chore: remove debug dep
mshanemc Jun 5, 2024
3810943
chore: don't export logger
mshanemc Jun 5, 2024
7a4c0ab
chore: logger renaming
mshanemc Jun 5, 2024
1351026
chore: typo
mshanemc Jun 5, 2024
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
8 changes: 5 additions & 3 deletions bin/dev.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
#!/usr/bin/env ts-node

async function main() {
const oclif = await import('@oclif/core');
oclif.settings.performanceEnabled = true;
await oclif.execute({ development: true, dir: import.meta.url });
const { settings } = await import('@oclif/core/settings');
const { execute } = await import('@oclif/core/execute');

settings.performanceEnabled = true;
await execute({ development: true, dir: import.meta.url });
}

await main();
33 changes: 8 additions & 25 deletions bin/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,13 @@
// Pre-process/prune flags before creating or running the actual CLI
(await import('../dist/flags.js')).preprocessCliFlags(process);

const oclif = await import('@oclif/core');
const { createRequire } = await import('module');
const pjson = createRequire(import.meta.url)('../package.json');
// Since the CLI is a single process, we can have a larger amount of max listeners since
// the process gets shut down. Don't set it to 0 (no limit) since we should still be aware
// of rouge event listeners
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// of rouge event listeners
// of rogue event listeners

process.setMaxListeners(parseInt(process.env.SF_MAX_EVENT_LISTENERS, 10) || 1000);

const cli = await import('../dist/cli.js');

async function main() {
// Since the CLI is a single process, we can have a larger amount of max listeners since
// the process gets shut down. Don't set it to 0 (no limit) since we should still be aware
// of rouge event listeners
process.setMaxListeners(parseInt(process.env.SF_MAX_EVENT_LISTENERS, 10) || 1000);

// Don't let other plugins override the CLI specified max listener count
process.setMaxListeners = () => {};
// Don't let other plugins override the CLI specified max listener count
process.setMaxListeners = () => {};

cli
.create({ version: pjson.version, bin: pjson.oclif.bin, channel: 'stable' })
.run()
.then(async () => {
await oclif.flush();
})
.catch(async (err) => {
await oclif.handle(err);
});
}

await main();
const cli = await import('../dist/cli.js');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it faster to await import? in both these cases we're going to do it anyway so, if not faster, maybe we just use regular imports

await cli.run();
10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@
},
"dependencies": {
"@inquirer/select": "^1.3.1",
"@oclif/core": "3.26.9",
"@oclif/core": "4.0.0",
"@oclif/plugin-autocomplete": "3.1.2",
"@oclif/plugin-commands": "4.0.2",
"@oclif/plugin-help": "6.1.0",
Expand Down Expand Up @@ -169,12 +169,12 @@
"@salesforce/plugin-templates": "56.2.9",
"@salesforce/plugin-trust": "3.7.4",
"@salesforce/plugin-user": "3.5.11",
"@salesforce/sf-plugins-core": "9.1.1",
"chalk": "^5.3.0",
"debug": "^4.3.4",
"strip-ansi": "^7.1.0"
"@salesforce/sf-plugins-core": "10.0.0",
"ansis": "^3.2.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it shipped, you can bump

"debug": "^4.3.4"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debug-js/debug#926 you might want the latest of debug for a recent fix

},
"pinnedDependencies": [
"@oclif/core",
"@oclif/plugin-autocomplete",
"@oclif/plugin-commands",
"@oclif/plugin-help",
Expand Down
70 changes: 22 additions & 48 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@
import { platform, arch, release } from 'node:os';
import { resolve } from 'node:path';
import { fileURLToPath } from 'node:url';
import { Config, Interfaces, run as oclifRun, settings } from '@oclif/core';
import { set } from '@salesforce/kit';
import Debug from 'debug';
import { default as nodeEnv, Env } from './util/env.js';

const debug = Debug('sf');
import { execute } from '@oclif/core/execute';
import { Config } from '@oclif/core/config';
import Interfaces from '@oclif/core/interfaces';
import NodeEnv, { Env } from './util/env.js';
import { logger } from './logger.js';

const envVars = [
...new Set([
Expand Down Expand Up @@ -41,14 +40,6 @@ export const UPDATE_DISABLED_DEMO =
'Manual and automatic CLI updates have been disabled in DEMO mode. ' +
'To check for a new version, unset the environment variable SF_ENV.';

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if nothing else is using env.getNpmRegistryOverride and env.setNpmRegistryOverride can you delete them?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and maybe the static prop on that class?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checking my understanding: the CLI itself doesn't need to know about this, so the plugins that do need it (ex: plugin trust) can get it when they do?

export function configureUpdateSites(config: Interfaces.Config, env = nodeEnv): void {
const npmRegistry = env.getNpmRegistryOverride();
if (npmRegistry) {
// Override config value if set via envar
set(config, 'pjson.oclif.warn-if-update-available.registry', npmRegistry);
}
}

export function configureAutoUpdate(envars: Env): void {
if (envars.isDemoMode()) {
// Disable autoupdates in demo mode
Expand Down Expand Up @@ -76,11 +67,12 @@ export function configureAutoUpdate(envars: Env): void {
}
}

function debugCliInfo(version: string, channel: string, env: Env, config: Interfaces.Config): void {
function debugCliInfo(env: Env, config: Interfaces.Config): void {
mshanemc marked this conversation as resolved.
Show resolved Hide resolved
function debugSection(section: string, items: Array<[string, string]>): void {
const pad = 25;
debug('%s:', section.padStart(pad));
items.forEach(([name, value]) => debug('%s: %s', name.padStart(pad), value));
const header = `### ${section} ###`;
logger.debug('%s', header.padStart(pad));
items.forEach(([name, value]) => logger.debug('%s: %s', name.padStart(pad), value));
}

debugSection('OS', [
Expand All @@ -93,8 +85,8 @@ function debugCliInfo(version: string, channel: string, env: Env, config: Interf
debugSection('NODE', [['version', process.versions.node]]);

debugSection('CLI', [
['version', version],
['channel', channel],
['version', config.version],
['channel', config.channel],
['bin', config.bin],
['data', config.dataDir],
['cache', config.cacheDir],
Expand All @@ -112,33 +104,15 @@ function debugCliInfo(version: string, channel: string, env: Env, config: Interf
);
}

type CreateOptions = {
version: string;
bin: string | undefined;
channel: string;
run?: typeof oclifRun;
env?: typeof nodeEnv;
};

export function create({ version, bin, channel, run, env }: CreateOptions): { run: () => Promise<unknown> } {
settings.performanceEnabled = true;
const root = resolve(fileURLToPath(import.meta.url), '..');
const args = process.argv.slice(2);
const environment = env ?? nodeEnv;
return {
async run(): Promise<unknown> {
const config = new Config({
name: bin,
root,
version,
channel,
});
await config.load();
configureUpdateSites(config, environment);
configureAutoUpdate(environment);
debugCliInfo(version, channel, environment, config);
// Example of how run is used in a test https://github.com/salesforcecli/cli/pull/171/files#diff-1deee0a575599b2df117c280da319f7938aaf6fdb0c04bcdbde769dbf464be69R46
return run ? run(args, config) : oclifRun(args, config);
},
};
export async function run(): Promise<unknown> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the part I don't understand (why we were doing it the other way before, what changed to make this the better option). It looks much cleaner, though!

configureAutoUpdate(NodeEnv);
const config = await Config.load({
root: resolve(fileURLToPath(import.meta.url), '..'),
logger,
enablePerf: true,
});
debugCliInfo(NodeEnv, config);
return execute({
loadOptions: config,
});
}
4 changes: 3 additions & 1 deletion src/help/sfCommandHelp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { Command, CommandHelp, HelpSection, HelpSectionRenderer, Interfaces } from '@oclif/core';
import { CommandHelp, HelpSection, HelpSectionRenderer } from '@oclif/core/help';
import { Command } from '@oclif/core/command';
import Interfaces from '@oclif/core/interfaces';
type SectionType = { header: string; generate: HelpSectionRenderer };

export class SfCommandHelp extends CommandHelp {
Expand Down
15 changes: 10 additions & 5 deletions src/help/sfHelp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { Command, CommandHelp, Help, Interfaces, toConfiguredId } from '@oclif/core';
import stripAnsi from 'strip-ansi';
import chalk from 'chalk';

import { Command } from '@oclif/core/command';
import { CommandHelp, Help } from '@oclif/core/help';
import Interfaces from '@oclif/core/interfaces';
import { toConfiguredId } from '@oclif/core/util/ids';
import { Ansis } from 'ansis';
import { SfCommandHelp } from './sfCommandHelp.js';

const ansis = new Ansis();

export default class SfHelp extends Help {
protected CommandHelpClass: typeof CommandHelp = SfCommandHelp;
protected commandHelpClass: SfCommandHelp | undefined;
Expand Down Expand Up @@ -62,9 +67,9 @@ export default class SfHelp extends Help {
protected log(...args: string[]): void {
const formatted = args.map((arg) => {
let formattedArg = arg.slice();
const matches = stripAnsi(formattedArg).match(this.commandIdRegex) ?? [];
const matches = ansis.strip(formattedArg).match(this.commandIdRegex) ?? [];
for (const match of matches) {
formattedArg = formattedArg.replaceAll(match, chalk.dim(match));
formattedArg = formattedArg.replaceAll(match, ansis.dim(match));
}

return formattedArg;
Expand Down
11 changes: 6 additions & 5 deletions src/hooks/display-release-notes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,19 @@
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { Hook, ux } from '@oclif/core';
import type { Hook } from '@oclif/core/hooks';
import ux from '@oclif/core/ux';

export const hook: Hook<'update'> = async function ({ config }) {
export const hook: Hook.Update = async function ({ config }) {
if (process.env.SF_HIDE_RELEASE_NOTES === 'true') return;

try {
return await config.runCommand('whatsnew', ['--hook']);
} catch (err) {
const error = err as Error;
ux.log('NOTE: This error can be ignored in CI and may be silenced in the future');
ux.log('- Set the SF_HIDE_RELEASE_NOTES env var to "true" to skip this script\n');
ux.log(error.message);
ux.stdout('NOTE: This error can be ignored in CI and may be silenced in the future');
ux.stdout('- Set the SF_HIDE_RELEASE_NOTES env var to "true" to skip this script\n');
ux.stdout(error.message);
}
};

Expand Down
8 changes: 6 additions & 2 deletions src/hooks/incomplete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import * as os from 'node:os';
import { Command, Hook, toConfiguredId, toStandardizedId, Interfaces, loadHelpClass } from '@oclif/core';
import os from 'node:os';
import { Command } from '@oclif/core/command';
import { type Hook } from '@oclif/core/hooks';
import { toConfiguredId, toStandardizedId } from '@oclif/core/util/ids';
import Interfaces from '@oclif/core/interfaces';
import { loadHelpClass } from '@oclif/core/help';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the help barrel isn't small and has a lot of big imports...maybe dynamically load this inside if (argv.includes('--help') || argv.includes('-h')) {


function buildChoices(
matches: Command.Loadable[],
Expand Down
5 changes: 3 additions & 2 deletions src/hooks/pluginsPreinstall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import { Hook, Errors } from '@oclif/core';
import { type Hook } from '@oclif/core/hooks';
import { handle } from '@oclif/core/handle';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe also only load this if needed inside the conditional instead of when the hook is instantiated?


const hook: Hook.PluginsPreinstall = async function (options) {
const verifySignHookResult = await this.config.runHook('plugins:preinstall:verify:signature', options);
Expand All @@ -14,7 +15,7 @@ const hook: Hook.PluginsPreinstall = async function (options) {
);

if (pluginTrustFailure !== undefined) {
await Errors.handle(pluginTrustFailure.error);
await handle(pluginTrustFailure.error);
}
};

Expand Down
4 changes: 2 additions & 2 deletions src/hooks/preparse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import type { Hook } from '@oclif/core';
import type { Hook } from '@oclif/core/hooks';

const hook: Hook<'preparse'> = async function ({ argv, options, context }) {
const hook: Hook.Preparse = async function ({ argv, options, context }) {
// Skip this hook if command does not have a --flags-dir flag or if it is not present in argv
if (!argv.includes('--flags-dir') || !options.flags?.['flags-dir']) return argv;
const flagsDir = argv[argv.indexOf('--flags-dir') + 1];
Expand Down
3 changes: 2 additions & 1 deletion src/hooks/prerun.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import { Hook, ux } from '@oclif/core';
import { type Hook } from '@oclif/core/hooks';
import ux from '@oclif/core/ux';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also maybe worth deferring to the conditional


// eslint-disable-next-line @typescript-eslint/require-await
const hook: Hook.Prerun = async function ({ Command, config }) {
Expand Down
24 changes: 24 additions & 0 deletions src/logger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright (c) 2023, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { format } from 'node:util';
import Interfaces from '@oclif/core/interfaces';
import { Logger } from '@salesforce/core';

mshanemc marked this conversation as resolved.
Show resolved Hide resolved
export const customLogger = (namespace: string): Interfaces.Logger => {
const sfLogger = new Logger(namespace);
return {
child: (ns: string, delimiter?: string) => customLogger(`${namespace}${delimiter ?? ':'}${ns}`),
debug: (formatter: unknown, ...args: unknown[]) => sfLogger.debug(format(formatter, ...args)),
mshanemc marked this conversation as resolved.
Show resolved Hide resolved
error: (formatter: unknown, ...args: unknown[]) => sfLogger.error(format(formatter, ...args)),
info: (formatter: unknown, ...args: unknown[]) => sfLogger.info(format(formatter, ...args)),
trace: (formatter: unknown, ...args: unknown[]) => sfLogger.trace(format(formatter, ...args)),
warn: (formatter: unknown, ...args: unknown[]) => sfLogger.warn(format(formatter, ...args)),
namespace,
};
};

export const logger = customLogger('sf');
Loading
Loading