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

fix: use ES2022 #793

Merged
merged 27 commits into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
933bb82
fix: use ES2022
mdonnalley Sep 22, 2023
4d14780
test: use --json for config unset
mdonnalley Sep 25, 2023
57794c3
feat: stop using getters and setters for flags
mdonnalley Sep 25, 2023
a893f34
chore: clean up
mdonnalley Sep 25, 2023
52f1b12
feat: expose json flag
mdonnalley Sep 26, 2023
178b9a9
feat: remove pass through getter and setter
mdonnalley Sep 26, 2023
f50ddf5
fix: correct order of flags in toCached
mdonnalley Sep 26, 2023
ea315ea
chore: clean up
mdonnalley Sep 26, 2023
f6db50f
fix: flag merge order
mdonnalley Sep 26, 2023
7d80d6f
chore: documentation
mdonnalley Sep 26, 2023
ea1a137
test: use oclif/test v3
mdonnalley Sep 26, 2023
a12dbf7
feat: set spinner style on windows too
mdonnalley Sep 26, 2023
45a777f
fix: handle cmd with baseFlags but no flags
mdonnalley Sep 26, 2023
bd8c6c8
fix: some circular deps
mdonnalley Sep 26, 2023
fc59ae2
fix: circular deps in help
mdonnalley Sep 26, 2023
cdff4b6
fix: ts-node and config circular deps
mdonnalley Sep 26, 2023
8195c03
fix: toCached circular dep in help
mdonnalley Sep 27, 2023
5a19e53
chore: organize utils
mdonnalley Sep 27, 2023
38d6d3d
test: enforce no circular deps
mdonnalley Sep 27, 2023
6d20d62
chore: remove Flags.json
mdonnalley Sep 27, 2023
ce3441f
chore: add prettier config
mdonnalley Sep 27, 2023
811efa1
test: add nyc
mdonnalley Sep 27, 2023
e365ad1
test: improve test coverage
mdonnalley Sep 27, 2023
920ea0e
test: windows unit tests
mdonnalley Sep 27, 2023
42282e4
chore: revert change to automerge.yml
mdonnalley Sep 27, 2023
09c9b66
chore: code review
mdonnalley Sep 28, 2023
f51da6b
perf: parallelize cacheCommand
mdonnalley Sep 28, 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
2 changes: 1 addition & 1 deletion .github/workflows/automerge.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ on:

jobs:
automerge:
uses: oclif/github-workflows/.github/workflows/automerge.yml@main
uses: salesforcecli/github-workflows/.github/workflows/automerge.yml@main
Copy link
Member

Choose a reason for hiding this comment

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

this won't work (secrets:inherit won't pass across github enterprise boundaries).

This would work (passing secrets explicitly)

secrets:
  SVC_CLI_BOT_GITHUB_TOKEN: {{secrets.SVC_CLI_BOT_GITHUB_TOKEN}

secrets: inherit
11 changes: 11 additions & 0 deletions .nycrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"check-coverage": true,
"lines": 80,
"statements": 70,
"functions": 70,
"branches": 60,
"reporter": ["lcov", "text"],
"extension": [".ts"],
"include": ["**/*.ts"],
"exclude": ["**/*.d.ts", "test/**"]
}
1 change: 1 addition & 0 deletions .prettierrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"@oclif/prettier-config"
45 changes: 45 additions & 0 deletions guides/PRE_CORE_MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ Migrating to `@oclif/core` from the deprecated oclif libraries (`@oclif/config`,

- [Migrating to @oclif/core from deprecated libraries](#migrating-to-oclifcore-from-deprecated-libraries)
- [Update Imports](#update-imports)
- [Update Command Args](#update-command-args)
- [Update your bin scripts](#update-your-bin-scripts)
- [Add `main` to your package.json](#add-main-to-your-packagejson)
- [Restore `-h`, `-v`, and `version`](#restore--h--v-and-version)
Expand All @@ -30,6 +31,50 @@ With this import:
import {Command, Flags, Topic, Help} from '@oclif/core';
```

## Update Command Args

We updated the `Command.args` to more closely resemble flags

**Before**

```typescript
import { Command } from '@oclif/core'

export default MyCommand extends Command {
static args = [{name: arg1, description: 'an argument', required: true}]

public async run(): Promise<void> {
const {args} = await this.parse(MyCommand) // args is useless {[name: string]: any}
}
}
```

**After**

```typescript
import { Command, Args } from '@oclif/core'

export default MyCommand extends Command {
static args = {
arg1: Args.string({description: 'an argument', required: true})
}

public async run(): Promise<void> {
const {args} = await this.parse(MyCommand) // args is { arg1: string }
}
}
```

These are the available Args:
- string
- integer
- boolean
- url
- file
- directory
- custom


## Update your bin scripts

`@oclif/core` now supports separate bin scripts for production and development.
Expand Down
27 changes: 24 additions & 3 deletions guides/V3_MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,18 @@ Migrating to @oclif/core@V3
- [BREAKING CHANGES ❗](#breaking-changes-)
- [Dropping node 14 and node 16 support](#dropping-node-14-and-node-16-support)
- [Bin Scripts for ESM/CJS Interoperability](#bin-scripts-for-esmcjs-interoperability)
- [Dropped `ts-node` as a dependency](#dropped-ts-node-as-a-dependency)
- [`Config.plugins`](#configplugins)
- [Readonly properties on `Config`](#readonly-properties-on-config)
- [Private methods on `Plugin`](#private-methods-on-plugin)
- [`global['cli-ux']` -\> `global.ux`](#globalcli-ux---globalux)
- [`handle`](#handle)
- [`noCacheDefault` flag property replaces `isWritingManifest`](#nocachedefault-flag-property-replaces-iswritingmanifest)
- [Features 🎉](#features-)
- [Cache Flexible taxonomy Command Permutations](#cache-flexible-taxonomy-command-permutations)
- [Performance Improvements](#performance-improvements)
- [charAliases Flag Property](#charaliases-flag-property)
- [Flags.option](#flagsoption)
- [Set spinner styles](#set-spinner-styles)


## BREAKING CHANGES ❗
Expand All @@ -36,6 +38,10 @@ In order to support ESM and CommonJS plugin interoperability you will need to up

If you'd like to migrate your plugin to ESM, please read our guide [here](https://oclif.io/docs/esm)

### Dropped `ts-node` as a dependency

We removed `ts-node` as a dependency to reduce the package size. By doing this, it means that linked plugin **must** have `ts-node` as a `devDependency` in order for auto-transpilation to work.

### `Config.plugins`
`Config.plugins` is now a `Map` where the keys are the plugin names and the values are the loaded `Plugin` instances. Previously it was an array of loaded `Plugin` instances.

Expand Down Expand Up @@ -113,9 +119,12 @@ export const mySensitiveFlag = Flags.string({

## Features 🎉

### Cache Flexible taxonomy Command Permutations
### Performance Improvements

The command permutations for flexible taxonomy are now cached in the oclif.manifest.json allowing for quicker startup times.
- Cache command permutations for flexible taxonomy in the `oclif.manifest.json`
- Cache additional command properties (`isESM`, `relativePath`) in the `oclif.manifest.json`
- Improved accuracy in the `DEBUG=perf` output.
- Remove `ts-node` from `dependencies` to reduce the package size.

### charAliases Flag Property

Expand Down Expand Up @@ -149,3 +158,15 @@ export default class MyCommand extends Command {
}
}
```

### Set spinner styles

You can now configure the style of the spinner when using `ux.action.start`. See [spinners](https://github.com/oclif/core/blob/main/src/cli-ux/action/spinners.ts) for all the different options.

```typescript
ux.action.start('starting spinner', 'spinning', {style: 'arc'})
await ux.wait(2500)
ux.action.status = 'still going'
await ux.wait(2500)
ux.action.stop()
```
12 changes: 8 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@
"@commitlint/config-conventional": "^12.1.4",
"@oclif/plugin-help": "^5.2.8",
"@oclif/plugin-plugins": "^3.3.0",
"@oclif/test": "^2.4.7",
"@oclif/prettier-config": "^0.1.1",
"@oclif/test": "^3.0.0-beta.1",
"@types/ansi-styles": "^3.2.1",
"@types/benchmark": "^2.1.2",
"@types/chai": "^4.3.4",
Expand Down Expand Up @@ -67,8 +68,10 @@
"fancy-test": "^3.0.0-beta.2",
"globby": "^11.1.0",
"husky": "6",
"madge": "^6.1.0",
Copy link
Member

Choose a reason for hiding this comment

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

+1 for madge

"mocha": "^10.2.0",
"nock": "^13.3.0",
"nyc": "^15.1.0",
"shx": "^0.3.4",
"sinon": "^11.1.2",
"tsd": "^0.29.0",
Expand Down Expand Up @@ -105,13 +108,14 @@
"commitlint": "commitlint",
"compile": "tsc",
"lint": "eslint . --ext .ts",
"posttest": "yarn lint",
"posttest": "yarn lint && yarn test:circular-deps",
"prepack": "yarn run build",
"pretest": "yarn build --noEmit && tsc -p test --noEmit --skipLibCheck",
"pretest": "yarn build && tsc -p test --noEmit --skipLibCheck",
"test:circular-deps": "madge lib/ -c",
"test:e2e": "mocha --forbid-only \"test/**/*.e2e.ts\" --parallel --timeout 1200000",
"test:esm-cjs": "cross-env DEBUG=e2e:* ts-node test/integration/esm-cjs.ts",
"test:perf": "ts-node test/perf/parser.perf.ts",
"test": "mocha --forbid-only \"test/**/*.test.ts\""
"test": "nyc mocha --forbid-only \"test/**/*.test.ts\""
},
"types": "lib/index.d.ts"
}
2 changes: 1 addition & 1 deletion src/cli-ux/action/spinner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export default class SpinnerAction extends ActionBase {
}

private getFrames(opts?: Options) {
if (opts?.style) return spinners[process.platform === 'win32' ? 'line' : opts.style].frames
if (opts?.style) return spinners[opts.style].frames

return spinners[process.platform === 'win32' ? 'line' : 'dots2'].frames
}
Expand Down
5 changes: 3 additions & 2 deletions src/cli-ux/flush.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {Errors, stdout} from '..'
import {error} from '../errors'
import {stdout} from './stream'

function timeout(p: Promise<any>, ms: number) {
function wait(ms: number, unref = false) {
Expand All @@ -8,7 +9,7 @@ function timeout(p: Promise<any>, ms: number) {
})
}

return Promise.race([p, wait(ms, true).then(() => Errors.error('timed out'))])
return Promise.race([p, wait(ms, true).then(() => error('timed out'))])
}

async function _flush() {
Expand Down
8 changes: 5 additions & 3 deletions src/cli-ux/styled/json.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
import chalk from 'chalk'
import {format} from 'node:util'
import {stdout} from '../stream'

import {ux} from '../../index'
const info = (output: string) => stdout.write(format(output) + '\n')

export default function styledJSON(obj: unknown): void {
const json = JSON.stringify(obj, null, 2)
if (!chalk.level) {
ux.info(json)
info(json)
return
}

const cardinal = require('cardinal')
const theme = require('cardinal/themes/jq')
ux.info(cardinal.highlight(json, {json: true, theme}))
info(cardinal.highlight(json, {json: true, theme}))
}
110 changes: 36 additions & 74 deletions src/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {LoadOptions} from './interfaces/config'
import {PJSON} from './interfaces'
import {Plugin} from './interfaces/plugin'
import {PrettyPrintableError} from './errors'
import {boolean} from './flags'
import {aggregateFlags} from './util/aggregate-flags'
import chalk from 'chalk'
import {fileURLToPath} from 'node:url'
import {ux} from './cli-ux'
Expand All @@ -42,13 +42,6 @@ stdout.on('error', (err: any) => {
throw err
})

const jsonFlag = {
json: boolean({
description: 'Format output as json.',
helpGroup: 'GLOBAL',
}),
}

/**
* An abstract class which acts as the base for each command
* in your project.
Expand Down Expand Up @@ -126,37 +119,7 @@ export abstract class Command {

public static hasDynamicHelp = false

protected static '_--' = false

protected static _enableJsonFlag = false

public static get enableJsonFlag(): boolean {
return this._enableJsonFlag
}

public static set enableJsonFlag(value: boolean) {
this._enableJsonFlag = value
if (value === true) {
this.baseFlags = jsonFlag
} else {
delete this.baseFlags?.json
this.flags = {} // force the flags setter to run
delete this.flags?.json
}
}

public static get '--'(): boolean {
return Command['_--']
}

public static set '--'(value: boolean) {
Command['_--'] = value
}

public get passThroughEnabled(): boolean {
return Command['_--']
}

public static enableJsonFlag = false
Copy link
Member

Choose a reason for hiding this comment

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

this whole file is a LOT better. I know all the complexity moved to the prototype crawler, but that's good for consumers.

/**
* instantiate and run the command
*
Expand Down Expand Up @@ -184,29 +147,10 @@ export abstract class Command {
return cmd._run<ReturnType<T['run']>>()
}

protected static _baseFlags: FlagInput

static get baseFlags(): FlagInput {
return this._baseFlags
}

static set baseFlags(flags: FlagInput) {
// eslint-disable-next-line prefer-object-spread
this._baseFlags = Object.assign({}, this.baseFlags, flags)
this.flags = {} // force the flags setter to run
}
public static baseFlags: FlagInput

/** A hash of flags for the command */
protected static _flags: FlagInput

public static get flags(): FlagInput {
return this._flags
}

public static set flags(flags: FlagInput) {
// eslint-disable-next-line prefer-object-spread
this._flags = Object.assign({}, this._flags ?? {}, this.baseFlags, flags)
}
public static flags: FlagInput

public id: string | undefined

Expand Down Expand Up @@ -284,16 +228,19 @@ export abstract class Command {
* @returns {boolean} true if the command supports json and the --json flag is present
*/
public jsonEnabled(): boolean {
// if the command doesn't support json, return false
// If the command doesn't support json, return false
if (!this.ctor.enableJsonFlag) return false
// if the command parameter pass through is enabled, return true if the --json flag is before the '--' separator
if (this.passThroughEnabled) {
const ptIndex = this.argv.indexOf('--')
const jsonIndex = this.argv.indexOf('--json')
return jsonIndex > -1 && (ptIndex === -1 || jsonIndex < ptIndex)
}

return this.argv.includes('--json') || this.config.scopedEnvVar?.('CONTENT_TYPE')?.toLowerCase() === 'json'
// If the CONTENT_TYPE env var is set to json, return true
if (this.config.scopedEnvVar?.('CONTENT_TYPE')?.toLowerCase() === 'json') return true

const passThroughIndex = this.argv.indexOf('--')
const jsonIndex = this.argv.indexOf('--json')
return passThroughIndex === -1
// If '--' is not present, then check for `--json` in this.argv
? jsonIndex > -1
// If '--' is present, return true only the --json flag exists and is before the '--'
: jsonIndex > -1 && jsonIndex < passThroughIndex
}

/**
Expand All @@ -312,8 +259,13 @@ export abstract class Command {
}

protected warnIfFlagDeprecated(flags: Record<string, unknown>): void {
const allFlags = aggregateFlags(
this.ctor.flags,
this.ctor.baseFlags,
this.ctor.enableJsonFlag,
)
for (const flag of Object.keys(flags)) {
const flagDef = this.ctor.flags[flag]
const flagDef = allFlags[flag]
const deprecated = flagDef?.deprecated
if (deprecated) {
this.warn(formatFlagDeprecationWarning(flag, deprecated))
Expand Down Expand Up @@ -352,12 +304,22 @@ export abstract class Command {
}
}

protected async parse<F extends FlagOutput, B extends FlagOutput, A extends ArgOutput>(options?: Input<F, B, A>, argv = this.argv): Promise<ParserOutput<F, B, A>> {
protected async parse<F extends FlagOutput, B extends FlagOutput, A extends ArgOutput>(
options?: Input<F, B, A>,
argv = this.argv,
): Promise<ParserOutput<F, B, A>> {
if (!options) options = this.ctor as Input<F, B, A>
const opts = {context: this, ...options}
// the spread operator doesn't work with getters so we have to manually add it here
opts.flags = options?.flags
opts.args = options?.args

const opts = {
context: this,
...options,
flags: aggregateFlags<F, B>(
options.flags,
options.baseFlags,
options.enableJsonFlag,
),
}

const results = await Parser.parse<F, B, A>(argv, opts)
this.warnIfFlagDeprecated(results.flags ?? {})

Expand Down
Loading
Loading