docs fixes

Author: reggi

docs/lib/build.js

@@ -376,7 +376,7 @@ const run = async (opts) => {
     const transformedSrc = applyTransforms(body, [
       transform.version,
       ...(fullName.startsWith('commands/')
-        ? [transform.usage, transform.params]
+        ? [transform.usage, transform.definitions]
         : []),
       ...(fullName === 'using-npm/config'
         ? [transform.shorthands, transform.config]

docs/lib/index.js

@@ -32,6 +32,27 @@ const getCommand = (name, commandLoader = defaultCommandLoader) => {
   return commandLoader(name)
 }
 
+// Resolve definitions for a command - use definitions if present, otherwise build from params
+const resolveDefinitions = (command) => {
+  // If command has definitions, use them directly (ignore params)
+  if (command.definitions && Object.keys(command.definitions).length > 0) {
+    return command.definitions
+  }
+
+  // Otherwise build from params using global definitions
+  if (command.params) {
+    const resolved = {}
+    for (const param of command.params) {
+      if (definitions[param]) {
+        resolved[param] = definitions[param]
+      }
+    }
+    return resolved
+  }
+
+  return {}
+}
+
 const getCommandByDoc = (docFile, docExt, commandLoader = defaultCommandLoader) => {
   // Grab the command name from the *.md filename
   // NOTE: We cannot use the name property command file because in the case of
@@ -41,7 +62,7 @@ const getCommandByDoc = (docFile, docExt, commandLoader = defaultCommandLoader)
   if (name === 'npm') {
     return {
       name,
-      params: null,
+      definitions: {},
       usage: 'npm',
     }
   }
@@ -51,19 +72,19 @@ const getCommandByDoc = (docFile, docExt, commandLoader = defaultCommandLoader)
   // so it just needs the usage of npm exec
   const srcName = name === 'npx' ? 'exec' : name
   const command = getCommand(srcName, commandLoader)
-  const { params, usage = [''], workspaces } = command
-  const commandDefinitions = command.definitions || {}
-  const definitionPool = { ...definitions, ...commandDefinitions }
+  const { usage = [''], workspaces } = command
   const usagePrefix = name === 'npx' ? 'npx' : `npm ${name}`
-  if (params) {
-    for (const param of params) {
-      // Check command-specific definitions first, fall back to global definitions
-      const paramDef = definitionPool[param]
-      if (paramDef && paramDef.exclusive) {
-        for (const e of paramDef.exclusive) {
-          if (!params.includes(e)) {
-            params.splice(params.indexOf(param) + 1, 0, e)
-          }
+
+  // Resolve definitions - handles exclusive params expansion
+  const commandDefs = resolveDefinitions(command)
+  const resolvedDefs = {}
+  for (const [key, def] of Object.entries(commandDefs)) {
+    resolvedDefs[key] = def
+    // Handle exclusive params
+    if (def.exclusive) {
+      for (const e of def.exclusive) {
+        if (!resolvedDefs[e] && definitions[e]) {
+          resolvedDefs[e] = definitions[e]
         }
       }
     }
@@ -72,7 +93,7 @@ const getCommandByDoc = (docFile, docExt, commandLoader = defaultCommandLoader)
   return {
     name,
     workspaces,
-    params: name === 'npx' ? null : params,
+    definitions: name === 'npx' ? {} : resolvedDefs,
     usage: usage.map(u => `${usagePrefix} ${u}`.trim()).join('\n'),
   }
 }
@@ -138,33 +159,30 @@ const generateFlagsTable = (definitionPool) => {
   ].join('\n')
 }
 
-const replaceParams = (src, { path, commandLoader }) => {
-  const { params, name } = getCommandByDoc(path, DOC_EXT, commandLoader)
+const replaceDefinitions = (src, { path, commandLoader }) => {
+  const { definitions: commandDefs, name } = getCommandByDoc(path, DOC_EXT, commandLoader)
 
-  // Load command to get command-specific definitions and subcommands if they exist
-  let commandDefinitions = {}
   let subcommands = {}
   try {
     const command = getCommand(name, commandLoader)
-    commandDefinitions = command.definitions || {}
     subcommands = command.subcommands || {}
   } catch {
-    // If command doesn't exist or has no definitions, continue with global definitions only
+    // Command doesn't exist
   }
 
-  // If no params and no subcommands, nothing to replace
-  if (!params && Object.keys(subcommands).length === 0) {
+  // If no definitions and no subcommands, nothing to replace
+  if (Object.keys(commandDefs).length === 0 && Object.keys(subcommands).length === 0) {
     return src
   }
 
-  // Assert placeholder is present - commands with params must have the config placeholder
+  // Assert placeholder is present
   const replacer = assertPlaceholder(src, path, TAGS.CONFIG)
 
   // If command has subcommands, generate sections for each subcommand
   if (Object.keys(subcommands).length > 0) {
     const subcommandSections = Object.entries(subcommands).map(([subName, SubCommand]) => {
       const subUsage = SubCommand.usage || []
-      const subDefinitions = SubCommand.definitions || {}
+      const subDefs = resolveDefinitions(SubCommand)
 
       const parts = [`### \`npm ${name} ${subName}\``, '']
 
@@ -181,10 +199,10 @@ const replaceParams = (src, { path, commandLoader }) => {
         parts.push('```', '')
       }
 
-      // Add flags section with all parameters combined
-      if (Object.keys(subDefinitions).length > 0) {
+      // Add flags section if definitions exist
+      if (Object.keys(subDefs).length > 0) {
         parts.push('#### Flags', '')
-        parts.push(generateFlagsTable(subDefinitions), '')
+        parts.push(generateFlagsTable(subDefs), '')
       }
 
       return parts.join('\n')
@@ -193,21 +211,10 @@ const replaceParams = (src, { path, commandLoader }) => {
     return src.replace(replacer, subcommandSections.join('\n'))
   }
 
-  // Original behavior for commands without subcommands but with params
-  /* istanbul ignore if - all commands with no subcommands have params */
-  if (!params) {
-    return src
-  }
-
-  // Build definitions object from params using global definitions pool
-  const allDefinitions = { ...definitions, ...commandDefinitions }
-  const paramDescriptions = []
-  for (const param of params) {
-    if (allDefinitions[param]) {
-      const def = allDefinitions[param]
-      paramDescriptions.push(def.describe())
-    }
-  }
+  // For commands without subcommands - commandDefs must be non-empty here
+  // (we would have returned early at line 175 if both were empty)
+  const paramDescriptions = Object.values(commandDefs)
+    .map(def => def.describe())
 
   return src.replace(replacer, paramDescriptions.join('\n\n'))
 }
@@ -284,7 +291,7 @@ module.exports = {
     md: resolve(__dirname, '..', 'content'),
   },
   usage: replaceUsage,
-  params: replaceParams,
+  definitions: replaceDefinitions,
   config: replaceConfig,
   shorthands: replaceShorthands,
   version: replaceVersion,

docs/test/index.js

@@ -808,6 +808,116 @@ description: Test command without params
     t.match(htmlContent, /npm testcmd-noparams/, 'includes command name')
   })
 
+  t.test('command with empty definitions (has config placeholder)', async t => {
+    const commandLoader = createCommandLoader({
+      'testcmd-empty-defs': {
+        usage: ['<pkg>'],
+        definitions: {},
+      },
+    })
+
+    const doc = createCommandDoc('npm-testcmd-empty-defs', 'Test command with empty definitions')
+    const { html } = await testBuildDocs(t, {
+      commandLoader,
+      content: {
+        commands: { 'npm-testcmd-empty-defs.md': doc },
+      },
+    })
+
+    const htmlContent = await readHtmlDoc(html, 'npm-testcmd-empty-defs')
+    t.ok(htmlContent.length > 0, 'generates HTML for command with empty definitions')
+    t.match(htmlContent, /npm testcmd-empty-defs/, 'includes command name')
+  })
+
+  t.test('command with params referencing non-existent global definition', async t => {
+    const commandLoader = createCommandLoader({
+      'testcmd-missing-param': {
+        usage: ['<pkg>'],
+        params: ['non-existent-param', 'registry'],
+      },
+    })
+
+    const doc = createCommandDoc('npm-testcmd-missing-param', 'Test command with missing param')
+    const { html } = await testBuildDocs(t, {
+      commandLoader,
+      content: {
+        commands: { 'npm-testcmd-missing-param.md': doc },
+      },
+    })
+
+    const htmlContent = await readHtmlDoc(html, 'npm-testcmd-missing-param')
+    t.ok(htmlContent.length > 0, 'generates HTML even with missing param definition')
+    t.match(htmlContent, /registry/, 'includes valid param')
+  })
+
+  t.test('command with exclusive param already resolved', async t => {
+    const commandLoader = createCommandLoader({
+      'testcmd-exclusive-resolved': {
+        usage: ['<pkg>'],
+        definitions: {
+          'save-dev': {
+            key: 'save-dev',
+            default: false,
+            type: Boolean,
+            description: 'Save as devDependency',
+            exclusive: ['save-optional'],
+            describe: () => '#### `save-dev`\n\n* Default: false\n* Type: Boolean\n\nSave as devDependency',
+          },
+          'save-optional': {
+            key: 'save-optional',
+            default: false,
+            type: Boolean,
+            description: 'Save as optionalDependency',
+            describe: () => '#### `save-optional`\n\n* Default: false\n* Type: Boolean\n\nSave as optionalDependency',
+          },
+        },
+      },
+    })
+
+    const doc = createCommandDoc('npm-testcmd-exclusive-resolved', 'Test command with exclusive already resolved')
+    const { html } = await testBuildDocs(t, {
+      commandLoader,
+      content: {
+        commands: { 'npm-testcmd-exclusive-resolved.md': doc },
+      },
+    })
+
+    const htmlContent = await readHtmlDoc(html, 'npm-testcmd-exclusive-resolved')
+    t.ok(htmlContent.length > 0, 'generates HTML with exclusive params')
+    t.match(htmlContent, /save-dev/, 'includes save-dev')
+    t.match(htmlContent, /save-optional/, 'includes save-optional')
+  })
+
+  t.test('command with exclusive param that does not exist in global definitions', async t => {
+    const commandLoader = createCommandLoader({
+      'testcmd-exclusive-missing': {
+        usage: ['<pkg>'],
+        definitions: {
+          'my-flag': {
+            key: 'my-flag',
+            default: false,
+            type: Boolean,
+            description: 'A flag with non-existent exclusive',
+            exclusive: ['non-existent-global-param'],
+            describe: () => '#### `my-flag`\n\n* Default: false\n* Type: Boolean\n\nA flag with non-existent exclusive',
+          },
+        },
+      },
+    })
+
+    const doc = createCommandDoc('npm-testcmd-exclusive-missing', 'Test command with missing exclusive')
+    const { html } = await testBuildDocs(t, {
+      commandLoader,
+      content: {
+        commands: { 'npm-testcmd-exclusive-missing.md': doc },
+      },
+    })
+
+    const htmlContent = await readHtmlDoc(html, 'npm-testcmd-exclusive-missing')
+    t.ok(htmlContent.length > 0, 'generates HTML even with missing exclusive param')
+    t.match(htmlContent, /my-flag/, 'includes the defined flag')
+  })
+
   t.test('command with one definition with short flag', async t => {
     const commandLoader = createCommandLoader({
       'testcmd-short': {
@@ -844,15 +954,14 @@ description: Test command without params
     const commandLoader = createCommandLoader({
       'testcmd-alias': {
         usage: ['<pkg>'],
-        params: ['aliased-flag'],
         definitions: {
           'aliased-flag': {
             key: 'aliased-flag',
             default: '',
             type: String,
             alias: ['af', 'alias-flag'],
             description: 'A flag with aliases',
-            describe: () => '#### `aliased-flag`\n\n* Default: ""\n* Type: String\n\nA flag with aliases',
+            describe: () => '#### `aliased-flag`\n\n* Default: ""\n* Type: String\n* Alias: --af, --alias-flag\n\nA flag with aliases',
           },
         },
       },
@@ -1131,10 +1240,11 @@ description: Test command without params
   })
 
   t.test('command with mixed command-specific and global params', async t => {
+    const { definitions: globalDefs } = require('@npmcli/config/lib/definitions')
+
     const commandLoader = createCommandLoader({
       'testcmd-mixed': {
         usage: ['<pkg>'],
-        params: ['custom-only', 'registry'],
         definitions: {
           'custom-only': {
             key: 'custom-only',
@@ -1143,6 +1253,7 @@ description: Test command without params
             description: 'A command-specific flag',
             describe: () => '#### `custom-only`\n\n* Default: false\n* Type: Boolean\n\nA command-specific flag',
           },
+          registry: globalDefs.registry,
         },
       },
     })
@@ -1162,10 +1273,11 @@ description: Test command without params
   })
 
   t.test('subcommand with command-specific and global params', async t => {
+    const { definitions: globalDefs } = require('@npmcli/config/lib/definitions')
+
     class SubMixed {
       static description = 'Subcommand with mixed params'
       static usage = ['<arg>']
-      static params = ['sub-flag', 'registry']
       static definitions = {
         'sub-flag': {
           key: 'sub-flag',
@@ -1174,6 +1286,7 @@ description: Test command without params
           description: 'A subcommand-specific flag',
           describe: () => '#### `sub-flag`\n\n* Default: false\n* Type: Boolean\n\nA subcommand-specific flag',
         },
+        registry: globalDefs.registry,
       }
     }
 
@@ -1312,14 +1425,19 @@ description: Test command without params
     class SubMixedNoDescribe {
       static description = 'Subcommand with command-specific params but no describe'
       static usage = ['<arg>']
-      static params = ['sub-custom', 'registry']
       static definitions = {
         'sub-custom': {
           key: 'sub-custom',
           default: 'default-val',
           type: String,
           description: 'A command-specific flag without describe',
         },
+        registry: {
+          key: 'registry',
+          default: 'https://registry.npmjs.org/',
+          type: String,
+          description: 'The base URL of the npm registry',
+        },
       }
     }
 

workspaces/config/test/index.js

@@ -1793,4 +1793,4 @@ t.test('before and min-release-age', async t => {
   await config.load()
   // Simple gut check to make sure we didn't do + instead of -
   t.ok(config.flat.before < Date.now(), 'before date is in the past not the future')
-})
+})