Oclif core migration #2144

k80bowman · 2022-11-16T16:00:27Z

Purpose

The purpose of this PR is to upgrade the Heroku CLI to use the newer @oclif/core utilities and remove dependencies on the following older, deprecated utilities:

@heroku-cli/dev-cli (a stop-gap repo to replace @oclif/dev-cli while we were completing this upgrade)
@oclif/command
@oclif/config
@oclif/dev-cli
@oclif/errors
@oclif/plugin-help
cli-ux

@oclif/core migration guide
GUS epic: Migrate to OCLIF core

Changes

Upgrading required Node version

@oclif/core requires that we use at least version 12 of Node. We had previously set the Node version for the packages at 10, so we needed to upgrade the minimum required version to 12. We are running the unit tests against versions 14 and 16 in the CI as well.

Types and Typescript

Previously, some of our variable types had come from the @oclif/config package. We had to switch these to use types from @oclif/core instead.

This upgrade required us to also upgrade our Typescript version from 3.7.5 to 4.8.4. This resulted in several necessary typing changes in the code.

Code changes

Tables

Several of our packages were using an earlier version of the cli-ux package. More recent versions had changed the parameters expected by the table() function. In order to upgrade to @oclif/core, we had to update the parameters we provide to the table() function to match the newer requirements.

Spinner fixes

In the apps and webhooks packages, we were having trouble with getting the unit tests to pass with the new CliUx.ux.action.start() and CliUx.ux.action.stop() functions. For some reason with these tests, the start() function was instantiating the spinner class before the stdout and stderr could be mocked. We updated several files in each of these packages to instantiate the spinner class directly, rather than going through the start() function, allowing the tests to pass. Internal folks who want more info on this can find it in this Slack thread.

Test expectations

The formats for some of the returns from @oclif/core functions changed a little, so we did need to update the expected returns in some of the unit tests.

Additional dependency removals and updates

This upgrade allowed us to also remove dependencies on:

@fancy-test/nock

Other necessary version updates across all packages include:

@heroku-cli/command (also updated to use @oclif/core)
@oclif/test
Additional dependency on the oclif package

Additional dependencies were also added to or upgraded in several individual packages as necessary.

nock - replaces @fancy-test/nock where needed
@typescript-eslint/eslint-plugin - added to autocomplete & pipelines packages
@typescript-eslint/parser - added to autocomplete & pipelines packages
@globby upgraded in several packages
Pipelines package
- pinned @types/chai to version 4.1.7
- pinned @types/mocha to 5.2.6
- pinned @types/node to 10.12.24
- pinned chai to 4.2.0
- pinned mocha to 5.2.0
- pinned ts-node to 10.9.1

jheikes515 · 2023-01-30T19:59:19Z

packages/auth/src/commands/auth/token.ts


 export default class AuthToken extends Command {
  static description = `outputs current CLI authentication token.
 By default, the CLI auth token is only valid for 1 year. To generate a long-lived token, use heroku authorizations:create`

-  static flags = {
+  static flags: Interfaces.FlagInput = {


Not sure we need this with the additionalHelpFlags specified in package.json for -h. otoh, it probably won't hurt anything to leave as is

eablack

Awesome work. Some questions:

Seems like a lot of API changes come with oclif. Will this change we're doing impact customer custom commands/packages that extend the heroku CLI? If so, do we need to do any customer communication about this?
We updated node to minimally require v12. Did we stop there because additional work is needed for v14? v12 was eol'd last year (https://github.com/nodejs/release#release-schedule).. I'm fine, btw, deferring getting to v14 to our other project workstream if that's what makes sense.
We've added a lot of awaits it seems. Did this impact the performance of commands at all?

packages/auth/test/test.ts

packages/autocomplete/src/commands/autocomplete/options.ts

jheikes515 · 2023-01-31T15:15:59Z

Awesome work. Some questions:

Seems like a lot of API changes come with oclif. Will this change we're doing impact customer custom commands/packages that extend the heroku CLI? If so, do we need to do any customer communication about this?

We updated node to minimally require v12. Did we stop there because additional work is needed for v14? v12 was eol'd last year (https://github.com/nodejs/release#release-schedule).. I'm fine, btw, deferring getting to v14 to our other project workstream if that's what makes sense.

We've added a lot of awaits it seems. Did this impact the performance of commands at all?

Thanks for the review!

Custom commands should be unchanged - we didn't remove anything other than outdated dependencies.
We wanted to keep other changes as minimal as possible - we plan to address further Node upgrades in another epic.
I have not noticed performance degradations from await - but we really had no choice where it's used, as oclif/core now requires it for some methods (e.g., parsing).

* wip: first step to remove all dependencies importing pre core. * chore: remove all imports of @oclif/core@2 * chore: remove @oclif/plugin-legacy and it's pre-core dependencies. * chore: remove confusing yarn version. Remove boostrap line from readme.

jheikes515 · 2023-01-31T16:53:04Z

packages/cli/test/acceptance/smoke.test.ts

@@ -52,7 +52,6 @@ describeOrSkip('@acceptance smoke tests', () => {
    const cmd = await run('plugins --core')
    expect(cmd.stdout).to.contain('@oclif/plugin-commands')
    expect(cmd.stdout).to.contain('@oclif/plugin-help')


Will these first two items actually be included in the output? They are both now part of oclif/core.

This reverts commit a1f4604.

* Revert "v8.0.1 (#2231)" This reverts commit d973836. * Revert "fix: don't run deb scripts in PR" This reverts commit 8fc7fc8. * Revert "fix: yarn downgrade for oclif linux release fixes (#2230)" This reverts commit d0f9234. * Revert "chore: add skipLibCheck to tsconfig for buildpacks and pipelines (#2229)" This reverts commit c8737ca. * Revert "chore: upgrade oclif version to 3.6.1 (#2228)" This reverts commit c5d476d. * Revert "chore: upgrade lerna and remove oclif readme command (#2227)" This reverts commit ed11841. * Revert "v8.0.0 (#2225)" This reverts commit 52372ab. * Revert "Oclif core migration (#2144)" This reverts commit a1f4604. * Revert "chore: fix snyk GH Action syntax (#2216)" This reverts commit 3175402. * Revert "refactor: Upgrade from yarn v1 to v3 (#2207)" This reverts commit 0589573. * Revert "move snyk to cron job (#2212)" This reverts commit d8711d5. * Revert "Revert "move snyk to cron job (#2212)"" This reverts commit b9f6885.

ryandagg temporarily deployed to AcceptanceTests November 17, 2022 22:27 Inactive

k80bowman temporarily deployed to AcceptanceTests November 22, 2022 14:30 Inactive

k80bowman temporarily deployed to AcceptanceTests November 23, 2022 14:09 Inactive

k80bowman temporarily deployed to AcceptanceTests November 23, 2022 14:10 Inactive

k80bowman temporarily deployed to AcceptanceTests November 23, 2022 14:11 Inactive

jheikes515 temporarily deployed to AcceptanceTests November 28, 2022 16:09 Inactive

k80bowman temporarily deployed to AcceptanceTests January 30, 2023 18:39 — with GitHub Actions Inactive

k80bowman had a problem deploying to AcceptanceTests January 30, 2023 18:39 — with GitHub Actions Failure

k80bowman temporarily deployed to AcceptanceTests January 30, 2023 18:39 — with GitHub Actions Inactive

k80bowman temporarily deployed to AcceptanceTests January 30, 2023 19:04 — with GitHub Actions Inactive

k80bowman temporarily deployed to AcceptanceTests January 30, 2023 19:05 — with GitHub Actions Inactive

k80bowman requested a review from eablack January 30, 2023 19:16

jheikes515 reviewed Jan 30, 2023

View reviewed changes

eablack reviewed Jan 30, 2023

View reviewed changes

packages/auth/test/test.ts Show resolved Hide resolved

packages/autocomplete/src/commands/autocomplete/options.ts Show resolved Hide resolved

ryandagg temporarily deployed to AcceptanceTests January 31, 2023 16:44 — with GitHub Actions Inactive

jheikes515 reviewed Jan 31, 2023

View reviewed changes

jheikes515 approved these changes Jan 31, 2023

View reviewed changes

ryandagg approved these changes Jan 31, 2023

View reviewed changes

ryandagg merged commit a1f4604 into master Jan 31, 2023

ryandagg deleted the oclif-core-migration branch January 31, 2023 17:47

ryandagg added a commit that referenced this pull request Feb 6, 2023

Revert "Oclif core migration (#2144)"

09dd927

This reverts commit a1f4604.

k80bowman mentioned this pull request Mar 15, 2023

Breaking Change: Oclif core migration #2270

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Oclif core migration #2144

Oclif core migration #2144

k80bowman commented Nov 16, 2022 •

edited

Loading

jheikes515 Jan 30, 2023 •

edited

Loading

eablack left a comment

jheikes515 commented Jan 31, 2023

jheikes515 Jan 31, 2023

Oclif core migration #2144

Oclif core migration #2144

Conversation

k80bowman commented Nov 16, 2022 • edited Loading

Purpose

Changes

Upgrading required Node version

Types and Typescript

Code changes

Tables

Spinner fixes

Test expectations

Additional dependency removals and updates

jheikes515 Jan 30, 2023 • edited Loading

Choose a reason for hiding this comment

eablack left a comment

Choose a reason for hiding this comment

jheikes515 commented Jan 31, 2023

jheikes515 Jan 31, 2023

Choose a reason for hiding this comment

k80bowman commented Nov 16, 2022 •

edited

Loading

jheikes515 Jan 30, 2023 •

edited

Loading