Skip to content

Commit

Permalink
fix(fund): correctly parse and use which config
Browse files Browse the repository at this point in the history 
Previously the `which` param in `npm fund` was validated incorrectly
leading to `EFUNDNUMBER` errors when used. This fixes that and does a
better job detecting when `which` is pointing to an incorrect array
bounds in the returned funding array.
  • Loading branch information
@lukekarrys
lukekarrys committed Jan 1, 2023
1 parent 72e6d6f commit 669ef94
Show file tree
Hide file tree
Showing 3 changed files with 443 additions and 396 deletions.
95 changes: 56 additions & 39 deletions lib/commands/fund.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,27 @@ const getPrintableName = ({ name, version }) => {
return `${name}${printableVersion}`
}

const errCode = (msg, code) => Object.assign(new Error(msg), { code })

class Fund extends ArboristWorkspaceCmd {
static description = 'Retrieve funding information'
static name = 'fund'
static params = ['json', 'browser', 'unicode', 'workspace', 'which']
static usage = ['[<package-spec>]']

// XXX: maybe worth making this generic for all commands?
usageMessage (paramsObj = {}) {
let msg = `\`npm ${this.constructor.name}`
const params = Object.entries(paramsObj)
if (params.length) {
msg += ` ${this.constructor.usage}`
}
for (const [key, value] of params) {
msg += ` --${key}=${value}`
}
return `${msg}\``
}

// TODO
/* istanbul ignore next */
async completion (opts) {
Expand All @@ -30,25 +45,23 @@ class Fund extends ArboristWorkspaceCmd {

async exec (args) {
const spec = args[0]
const numberArg = this.npm.config.get('which')

const fundingSourceNumber = numberArg && parseInt(numberArg, 10)

const badFundingSourceNumber =
numberArg !== null && (String(fundingSourceNumber) !== numberArg || fundingSourceNumber < 1)

if (badFundingSourceNumber) {
const err = new Error(
'`npm fund [<@scope>/]<pkg> [--which=fundingSourceNumber]` must be given a positive integer'
)
err.code = 'EFUNDNUMBER'
throw err
let fundingSourceNumber = this.npm.config.get('which')
if (fundingSourceNumber != null) {
fundingSourceNumber = parseInt(fundingSourceNumber, 10)
if (isNaN(fundingSourceNumber) || fundingSourceNumber < 1) {
throw errCode(
`${this.usageMessage({ which: 'fundingSourceNumber' })} must be given a positive integer`,
'EFUNDNUMBER'
)
}
}

if (this.npm.global) {
const err = new Error('`npm fund` does not support global packages')
err.code = 'EFUNDGLOBAL'
throw err
throw errCode(
`${this.usageMessage()} does not support global packages`,
'EFUNDGLOBAL'
)
}

const where = this.npm.prefix
Expand Down Expand Up @@ -146,6 +159,7 @@ class Fund extends ArboristWorkspaceCmd {

async openFundingUrl ({ path, tree, spec, fundingSourceNumber }) {
const arg = npa(spec, path)

const retrievePackageMetadata = () => {
if (arg.type === 'directory') {
if (tree.path === arg.fetchSpec) {
Expand Down Expand Up @@ -178,32 +192,35 @@ class Fund extends ArboristWorkspaceCmd {

const validSources = [].concat(normalizeFunding(funding)).filter(isValidFunding)

const matchesValidSource =
validSources.length === 1 ||
(fundingSourceNumber > 0 && fundingSourceNumber <= validSources.length)

if (matchesValidSource) {
const index = fundingSourceNumber ? fundingSourceNumber - 1 : 0
const { type, url } = validSources[index]
const typePrefix = type ? `${type} funding` : 'Funding'
const msg = `${typePrefix} available at the following URL`
return openUrl(this.npm, url, msg)
} else if (validSources.length && !(fundingSourceNumber >= 1)) {
validSources.forEach(({ type, url }, i) => {
const typePrefix = type ? `${type} funding` : 'Funding'
const msg = `${typePrefix} available at the following URL`
this.npm.output(`${i + 1}: ${msg}: ${url}`)
})
this.npm.output(
/* eslint-disable-next-line max-len */
'Run `npm fund [<@scope>/]<pkg> --which=1`, for example, to open the first funding URL listed in that package'
)
} else {
const noFundingError = new Error(`No valid funding method available for: ${spec}`)
noFundingError.code = 'ENOFUND'
if (!validSources.length) {
throw errCode(`No valid funding method available for: ${spec}`, 'ENOFUND')
}

throw noFundingError
const fundSource = fundingSourceNumber
? validSources[fundingSourceNumber - 1]
: validSources.length === 1 ? validSources[0]
: null

if (fundSource) {
return openUrl(this.npm, ...this.urlMessage(fundSource))
}

const ambiguousUrlMsg = [
...validSources.map((s, i) => `${i + 1}: ${this.urlMessage(s).reverse().join(': ')}`),
`Run ${this.usageMessage({ which: '1' })}` +
', for example, to open the first funding URL listed in that package',
]
if (fundingSourceNumber) {
ambiguousUrlMsg.unshift(`--which=${fundingSourceNumber} is not a valid index`)
}
this.npm.output(ambiguousUrlMsg.join('\n'))
}

urlMessage (source) {
const { type, url } = source
const typePrefix = type ? `${type} funding` : 'Funding'
const message = `${typePrefix} available at the following URL`
return [url, message]
}
}
module.exports = Fund
44 changes: 11 additions & 33 deletions tap-snapshots/test/lib/commands/fund.js.test.cjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
exports[`test/lib/commands/fund.js TAP fund a package with type and multiple sources > should print prompt select message 1`] = `
1: Foo funding available at the following URL: http://example.com/foo
2: Lorem funding available at the following URL: http://example.com/foo-lorem
Run \`npm fund [<@scope>/]<pkg> --which=1\`, for example, to open the first funding URL listed in that package
Run \`npm fund [<package-spec>] --which=1\`, for example, to open the first funding URL listed in that package
`

exports[`test/lib/commands/fund.js TAP fund colors > should print output with color info 1`] = `
Expand All @@ -23,7 +22,6 @@ exports[`test/lib/commands/fund.js TAP fund colors > should print output with co
 \`-- http://example.com/e
 \`-- e@1.0.0

`

exports[`test/lib/commands/fund.js TAP fund containing multi-level nested deps with no funding > should omit dependencies with no funding declared 1`] = `
Expand All @@ -33,54 +31,37 @@ nested-no-funding-packages@1.0.0
\`-- http://example.com/donate
\`-- bar@1.0.0
`

exports[`test/lib/commands/fund.js TAP fund in which same maintainer owns all its deps > should print stack packages together 1`] = `
http://example.com/donate
\`-- maintainer-owns-all-deps@1.0.0, dep-foo@1.0.0, dep-sub-foo@1.0.0, dep-bar@1.0.0
`

exports[`test/lib/commands/fund.js TAP fund pkg missing version number > should print name only 1`] = `
http://example.com/foo
\`-- foo
`

exports[`test/lib/commands/fund.js TAP fund using bad which value: index too high > should print message about invalid which 1`] = `
--which=100 is not a valid index
1: Funding available at the following URL: http://example.com
2: Funding available at the following URL: http://sponsors.example.com/me
3: Funding available at the following URL: http://collective.example.com
Run \`npm fund [<package-spec>] --which=1\`, for example, to open the first funding URL listed in that package
`

exports[`test/lib/commands/fund.js TAP fund using nested packages with multiple sources > should prompt with all available URLs 1`] = `
1: Funding available at the following URL: https://one.example.com
2: Funding available at the following URL: https://two.example.com
Run \`npm fund [<@scope>/]<pkg> --which=1\`, for example, to open the first funding URL listed in that package
`

exports[`test/lib/commands/fund.js TAP fund using nested packages with multiple sources, with a source number > should open the numbered URL 1`] = `
Funding available at the following URL:
https://one.example.com
`

exports[`test/lib/commands/fund.js TAP fund using package argument > should open funding url 1`] = `
individual funding available at the following URL:
http://example.com/donate
`

exports[`test/lib/commands/fund.js TAP fund using pkg name while having conflicting versions > should open greatest version 1`] = `
Funding available at the following URL:
http://example.com/2
`

exports[`test/lib/commands/fund.js TAP fund using string shorthand > should open string-only url 1`] = `
Funding available at the following URL:
https://example.com/sponsor
Run \`npm fund [<package-spec>] --which=1\`, for example, to open the first funding URL listed in that package
`

exports[`test/lib/commands/fund.js TAP fund with no package containing funding > should print empty funding info 1`] = `
no-funding-package@0.0.0
`

exports[`test/lib/commands/fund.js TAP sub dep with fund info and a parent with no funding info > should nest sub dep as child of root 1`] = `
Expand All @@ -90,25 +71,22 @@ test-multiple-funding-sources@1.0.0
\`-- http://example.com/c
\`-- c@1.0.0
`

exports[`test/lib/commands/fund.js TAP workspaces filter funding info by a specific workspace > should display only filtered workspace name and its deps 1`] = `
exports[`test/lib/commands/fund.js TAP workspaces filter funding info by a specific workspace name > should display only filtered workspace name and its deps 1`] = `
workspaces-support@1.0.0
\`-- https://example.com/a
| \`-- a@1.0.0
\`-- http://example.com/c
\`-- c@1.0.0
`

exports[`test/lib/commands/fund.js TAP workspaces filter funding info by a specific workspace > should display only filtered workspace path and its deps 1`] = `
exports[`test/lib/commands/fund.js TAP workspaces filter funding info by a specific workspace path > should display only filtered workspace name and its deps 1`] = `
workspaces-support@1.0.0
\`-- https://example.com/a
| \`-- a@1.0.0
\`-- http://example.com/c
\`-- c@1.0.0
`
Loading

0 comments on commit 669ef94

Please sign in to comment.