Merge pull request #1681 from oclif/mdonnalley/1680

fix: ensure jit plugins are downloaded via npm

src/commands/manifest.ts

@@ -1,13 +1,8 @@
 import {Args, Command, Flags, Interfaces, Plugin, ux} from '@oclif/core'
-import {access, createWriteStream, mkdir, readJSON, readJSONSync, remove, unlinkSync, writeFileSync} from 'fs-extra'
+import {access, mkdir, readJSON, readJSONSync, remove, unlinkSync, writeFileSync} from 'fs-extra'
 import {exec, ExecOptions} from 'node:child_process'
 import * as os from 'node:os'
 import path from 'node:path'
-import {pipeline as pipelineSync} from 'node:stream'
-import {promisify} from 'node:util'
-import {maxSatisfying} from 'semver'
-
-const pipeline = promisify(pipelineSync)
 
 async function fileExists(filePath: string): Promise<boolean> {
   try {
@@ -40,27 +35,20 @@ export default class Manifest extends Command {
     const {args} = await this.parse(Manifest)
     const root = path.resolve(args.path)
 
-    const packageJson = readJSONSync('package.json') as Interfaces.PJSON
-
+    const packageJson = readJSONSync(path.join(root, 'package.json')) as Interfaces.PJSON
     let jitPluginManifests: Interfaces.Manifest[] = []
 
     if (flags.jit && packageJson.oclif?.jitPlugins) {
       this.debug('jitPlugins: %s', packageJson.oclif.jitPlugins)
       const tmpDir = os.tmpdir()
-      const {default: got} = await import('got')
       const promises = Object.entries(packageJson.oclif.jitPlugins).map(async ([jitPlugin, version]) => {
         const pluginDir = jitPlugin.replace('/', '-').replace('@', '')
-
         const fullPath = path.join(tmpDir, pluginDir)
 
         if (await fileExists(fullPath)) await remove(fullPath)
-
         await mkdir(fullPath, {recursive: true})
 
-        const resolvedVersion = await this.getVersion(jitPlugin, version)
-        const tarballUrl = await this.getTarballUrl(jitPlugin, resolvedVersion)
-        const tarball = path.join(fullPath, path.basename(tarballUrl))
-        await pipeline(got.stream(tarballUrl), createWriteStream(tarball))
+        const tarball = await this.downloadTarball(jitPlugin, version, fullPath)
 
         await this.executeCommand(`tar -xzf "${tarball}"`, {cwd: fullPath})
 
@@ -117,6 +105,25 @@ export default class Manifest extends Command {
     return plugin.manifest
   }
 
+  private async downloadTarball(plugin: string, version: string, tarballStoragePath: string): Promise<string> {
+    const {stderr} = await this.executeCommand(
+      `npm pack ${plugin}@${version} --pack-destination "${tarballStoragePath}" --json`,
+    )
+    // You can `npm pack` with multiple modules to download multiple at a time. There will be at least 1 if the command
+    // succeeded.
+    const tarballs = JSON.parse(stderr) as {
+      filename: string
+    }[]
+
+    if (!Array.isArray(tarballs) || tarballs.length !== 1) {
+      throw new Error(`Could not download tarballs for ${plugin}. Tarball download was not in the correct format.`)
+    }
+
+    const {filename} = tarballs[0]
+
+    return path.join(tarballStoragePath, filename)
+  }
+
   private async executeCommand(command: string, options?: ExecOptions): Promise<{stderr: string; stdout: string}> {
     return new Promise((resolve) => {
       exec(command, options, (error, stderr, stdout) => {
@@ -131,31 +138,4 @@ export default class Manifest extends Command {
       })
     })
   }
-
-  private async getTarballUrl(plugin: string, version: string): Promise<string> {
-    const {stderr} = await this.executeCommand(`npm view ${plugin}@${version} --json`)
-    const {dist} = JSON.parse(stderr) as {
-      dist: {tarball: string}
-    }
-    return dist.tarball
-  }
-
-  private async getVersion(plugin: string, version: string): Promise<string> {
-    if (version.startsWith('^') || version.startsWith('~')) {
-      // Grab latest from npm to get all the versions so we can find the max satisfying version.
-      // We explicitly ask for latest since this command is typically run inside of `npm prepack`,
-      // which sets the npm_config_tag env var, which is used as the default anytime a tag isn't
-      // provided to `npm view`. This can be problematic if you're building the `nightly` version
-      // of a CLI and all the JIT plugins don't have a `nightly` tag themselves.
-      // TL;DR - always ask for latest to avoid potentially requesting a non-existent tag.
-      const {stderr} = await this.executeCommand(`npm view ${plugin}@latest --json`)
-      const {versions} = JSON.parse(stderr) as {
-        versions: string[]
-      }
-
-      return maxSatisfying(versions, version) ?? version.replace('^', '').replace('~', '')
-    }
-
-    return version
-  }
 }

test/fixtures/cli-with-jit-plugin/README.md

@@ -0,0 +1,16 @@
+# cli-with-jit-plugin
+
+This file is a test for running `oclif manifest` with jit plugins.
+
+<!-- toc -->
+<!-- tocstop -->
+
+# Usage
+
+<!-- usage -->
+<!-- usagestop -->
+
+# Commands
+
+<!-- commands -->
+<!-- commandsstop -->

test/fixtures/cli-with-jit-plugin/package.json

@@ -0,0 +1,14 @@
+{
+  "name": "cli-with-jit-plugin",
+  "files": [
+    "/lib"
+  ],
+  "oclif": {
+    "commands": "./lib/commands",
+    "bin": "oclif",
+    "helpClass": "./lib/help",
+    "jitPlugins": {
+      "oclif": "*"
+    }
+  }
+}

test/fixtures/cli-with-jit-plugin/src/commands/hello.ts

@@ -0,0 +1,10 @@
+import {Command} from '@oclif/core'
+
+export default class Hello extends Command {
+  static description = 'a simple command'
+  static flags = {}
+
+  async run(): Promise<void> {
+    this.log('hello world')
+  }
+}

test/fixtures/cli-with-jit-plugin/src/help.ts

@@ -0,0 +1,7 @@
+import {Command, Help} from '@oclif/core'
+
+export default class CustomHelp extends Help {
+  formatCommand(command: Command.Class): string {
+    return `Custom help for ${command.id}`
+  }
+}

test/fixtures/cli-with-jit-plugin/tsconfig.json

@@ -0,0 +1,12 @@
+{
+  "compilerOptions": {
+    "declaration": true,
+    "importHelpers": true,
+    "module": "commonjs",
+    "outDir": "lib",
+    "rootDir": "src",
+    "strict": true,
+    "target": "es2017"
+  },
+  "include": ["src/**/*"]
+}

test/fixtures/cli-with-nested-topics-with-space-separator/package.json

@@ -9,9 +9,9 @@
     "helpClass": "./lib/help",
     "topicSeparator": " ",
     "topics": {
-      "roottopic":{
-          "description": "Root topic description",
-          "hidden":true
+      "roottopic": {
+        "description": "Root topic description",
+        "hidden": true
       },
       "roottopic:subtopic1": {
         "description": "Subtopic1 description"

test/fixtures/cli-with-nested-topics-with-space-separator/tsconfig.json

@@ -8,7 +8,5 @@
     "strict": true,
     "target": "es2017"
   },
-  "include": [
-    "src/**/*"
-  ]
+  "include": ["src/**/*"]
 }

test/fixtures/cli-with-nested-topics/package.json

@@ -8,9 +8,9 @@
     "bin": "oclif",
     "helpClass": "./lib/help",
     "topics": {
-      "roottopic":{
-          "description": "Root topic description",
-          "hidden":true
+      "roottopic": {
+        "description": "Root topic description",
+        "hidden": true
       },
       "roottopic:subtopic1": {
         "description": "Subtopic1 description"

test/fixtures/cli-with-nested-topics/tsconfig.json

@@ -8,7 +8,5 @@
     "strict": true,
     "target": "es2017"
   },
-  "include": [
-    "src/**/*"
-  ]
+  "include": ["src/**/*"]
 }

test/integration/sf.test.ts

@@ -38,7 +38,7 @@ describe('sf', () => {
 
       const manifest = JSON.parse(await readFile(join(sfDir, 'oclif.manifest.json'), 'utf8')) as Interfaces.Manifest
 
-      const sfPjson = JSON.parse(await readFile(join(sfDir, 'package.json'), 'utf8')) as Interfaces.PJSON.Plugin
+      const sfPjson = JSON.parse(await readFile(join(sfDir, 'package.json'), 'utf8')) as Interfaces.PJSON['Plugin']
       const jitPlugins = Object.keys(sfPjson.oclif.jitPlugins ?? {})
 
       const everyPluginHasCommand = jitPlugins.every((jitPlugin) =>

test/unit/manifest.test.ts

@@ -2,14 +2,19 @@ import {Interfaces} from '@oclif/core'
 import {runCommand} from '@oclif/test'
 import {expect} from 'chai'
 import {rm} from 'node:fs/promises'
+import path from 'node:path'
 
 describe('manifest', () => {
+  let cwd: string
+
   beforeEach(async () => {
+    cwd = process.cwd()
     await rm('oclif.manifest.json', {force: true})
   })
 
   afterEach(async () => {
     await rm('oclif.manifest.json', {force: true})
+    process.chdir(cwd)
   })
 
   it('should generate manifest', async () => {
@@ -19,4 +24,23 @@ describe('manifest', () => {
       description: 'Generates plugin manifest json (oclif.manifest.json).',
     })
   })
+
+  it('should generate jit plugins', async () => {
+    const {error, result: manifest} = await runCommand<Interfaces.Manifest>(
+      `manifest ${path.join(__dirname, '../fixtures/cli-with-jit-plugin')}`,
+    )
+    const jitPlugins = ['oclif']
+
+    const everyPluginHasCommand = jitPlugins.every((jitPlugin) =>
+      Boolean(Object.values(manifest?.commands ?? []).some((command) => command.pluginName === jitPlugin)),
+    )
+    const everyJITCommandIsTypeJIT = Object.values(manifest?.commands ?? [])
+      .filter((command) => jitPlugins.includes(command.pluginName ?? ''))
+      .every((command) => command.pluginType === 'jit')
+
+    expect(everyPluginHasCommand).to.be.true
+    expect(everyJITCommandIsTypeJIT).to.be.true
+    expect(manifest?.commands.hello).to.be.ok
+    expect(error).to.be.undefined
+  })
 })