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(log): pass in logger to external modules #4359

Merged
merged 1 commit into from
Feb 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion lib/commands/access.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const path = require('path')
const libaccess = require('libnpmaccess')
const readPackageJson = require('read-package-json-fast')

const log = require('../utils/log-shim.js')
const otplease = require('../utils/otplease.js')
const getIdentity = require('../utils/get-identity.js')
const BaseCommand = require('../base-command.js')
Expand Down Expand Up @@ -76,7 +77,10 @@ class Access extends BaseCommand {
throw this.usageError(`${cmd} is not a recognized subcommand.`)
}

return this[cmd](args, this.npm.flatOptions)
return this[cmd](args, {
...this.npm.flatOptions,
log,
})
}

public ([pkg], opts) {
Expand Down
3 changes: 3 additions & 0 deletions lib/commands/deprecate.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const fetch = require('npm-registry-fetch')
const log = require('../utils/log-shim.js')
const otplease = require('../utils/otplease.js')
const npa = require('npm-package-arg')
const semver = require('semver')
Expand Down Expand Up @@ -50,6 +51,7 @@ class Deprecate extends BaseCommand {
...this.npm.flatOptions,
spec: p,
query: { write: true },
log,
})

Object.keys(packument.versions)
Expand All @@ -64,6 +66,7 @@ class Deprecate extends BaseCommand {
method: 'PUT',
body: packument,
ignoreBody: true,
log,
}))
}
}
Expand Down
2 changes: 2 additions & 0 deletions lib/commands/diff.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class Diff extends BaseCommand {
...this.npm.flatOptions,
diffFiles: args,
where: this.top,
log,
})
return this.npm.output(res)
}
Expand Down Expand Up @@ -193,6 +194,7 @@ class Diff extends BaseCommand {
const packument = await pacote.packument(spec, {
...this.npm.flatOptions,
preferOnline: true,
log,
})
bSpec = pickManifest(
packument,
Expand Down
5 changes: 4 additions & 1 deletion lib/commands/dist-tag.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ class DistTag extends BaseCommand {
}

async exec ([cmdName, pkg, tag]) {
const opts = this.npm.flatOptions
const opts = {
...this.npm.flatOptions,
log,
}

if (['add', 'a', 'set', 's'].includes(cmdName)) {
return this.add(pkg, tag, opts)
Expand Down
6 changes: 5 additions & 1 deletion lib/commands/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const hookApi = require('libnpmhook')
const otplease = require('../utils/otplease.js')
const relativeDate = require('tiny-relative-date')
const Table = require('cli-table3')
const log = require('../utils/log-shim.js')

const BaseCommand = require('../base-command.js')
class Hook extends BaseCommand {
Expand All @@ -20,7 +21,10 @@ class Hook extends BaseCommand {
]

async exec (args) {
return otplease(this.npm.flatOptions, (opts) => {
return otplease({
...this.npm.flatOptions,
log,
}, (opts) => {
switch (args[0]) {
case 'add':
return this.add(args[1], args[2], args[3], opts)
Expand Down
1 change: 1 addition & 0 deletions lib/commands/logout.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class Logout extends BaseCommand {
...this.npm.flatOptions,
method: 'DELETE',
ignoreBody: true,
log,
})
} else if (auth.isBasicAuth) {
log.verbose('logout', `clearing user credentials for ${reg}`)
Expand Down
6 changes: 5 additions & 1 deletion lib/commands/owner.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ class Owner extends BaseCommand {
}

async exec ([action, ...args]) {
const opts = this.npm.flatOptions
const opts = {
...this.npm.flatOptions,
log,
}
switch (action) {
case 'ls':
case 'list':
Expand Down Expand Up @@ -195,6 +198,7 @@ class Owner extends BaseCommand {
method: 'PUT',
body,
spec,
log,
})
})

Expand Down
2 changes: 1 addition & 1 deletion lib/commands/ping.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class Ping extends BaseCommand {
async exec (args) {
log.notice('PING', this.npm.config.get('registry'))
const start = Date.now()
const details = await pingUtil(this.npm.flatOptions)
const details = await pingUtil({ ...this.npm.flatOptions, log })
const time = Date.now() - start
log.notice('PONG', `${time}ms`)
if (this.npm.config.get('json')) {
Expand Down
8 changes: 4 additions & 4 deletions lib/commands/profile.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ class Profile extends BaseCommand {
async get (args) {
const tfa = 'two-factor auth'
const info = await pulseTillDone.withPromise(
npmProfile.get(this.npm.flatOptions)
npmProfile.get({ ...this.npm.flatOptions, log })
)

if (!info.cidr_whitelist) {
Expand Down Expand Up @@ -170,7 +170,7 @@ class Profile extends BaseCommand {
}

async set (args) {
const conf = this.npm.flatOptions
const conf = { ...this.npm.flatOptions, log }
const prop = (args[0] || '').toLowerCase().trim()

let value = args.length > 1 ? args.slice(1).join(' ') : null
Expand Down Expand Up @@ -285,7 +285,7 @@ class Profile extends BaseCommand {
if (auth.basic) {
log.info('profile', 'Updating authentication to bearer token')
const result = await npmProfile.createToken(
auth.basic.password, false, [], this.npm.flatOptions
auth.basic.password, false, [], { ...this.npm.flatOptions, log }
)

if (!result.token) {
Expand All @@ -309,7 +309,7 @@ class Profile extends BaseCommand {

log.info('profile', 'Determine if tfa is pending')
const userInfo = await pulseTillDone.withPromise(
npmProfile.get(this.npm.flatOptions)
npmProfile.get({ ...this.npm.flatOptions, log })
)

const conf = { ...this.npm.flatOptions }
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class Publish extends BaseCommand {
throw new Error('Tag name must not be a valid SemVer range: ' + defaultTag.trim())
}

const opts = { ...this.npm.flatOptions }
const opts = { ...this.npm.flatOptions, log }

// you can publish name@version, ./foo.tgz, etc.
// even though the default is the 'file:.' cwd.
Expand Down
4 changes: 3 additions & 1 deletion lib/commands/star.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,13 @@ class Star extends BaseCommand {
const pkgs = args.map(npa)
for (const pkg of pkgs) {
const [username, fullData] = await Promise.all([
getIdentity(this.npm, this.npm.flatOptions),
getIdentity(this.npm, { ...this.npm.flatOptions, log }),
fetch.json(pkg.escapedName, {
...this.npm.flatOptions,
spec: pkg,
query: { write: true },
preferOnline: true,
log,
}),
])

Expand Down Expand Up @@ -63,6 +64,7 @@ class Star extends BaseCommand {
spec: pkg,
method: 'PUT',
body,
log,
})

this.npm.output(show + ' ' + pkg.name)
Expand Down
3 changes: 2 additions & 1 deletion lib/commands/team.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const columns = require('cli-columns')
const libteam = require('libnpmteam')

const log = require('../utils/log-shim.js')
const otplease = require('../utils/otplease.js')

const BaseCommand = require('../base-command.js')
Expand Down Expand Up @@ -42,7 +43,7 @@ class Team extends BaseCommand {
// XXX: "description" option to libnpmteam is used as a description of the
// team, but in npm's options, this is a boolean meaning "show the
// description in npm search output". Hence its being set to null here.
await otplease(this.npm.flatOptions, opts => {
await otplease({ ...this.npm.flatOptions, log }, opts => {
entity = entity.replace(/^@/, '')
switch (cmd) {
case 'create': return this.create(entity, opts)
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/token.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class Token extends BaseCommand {
}

config () {
const conf = { ...this.npm.flatOptions }
const conf = { ...this.npm.flatOptions, log }
const creds = this.npm.config.getCredentialsByURI(conf.registry)
if (creds.token) {
conf.auth = { token: creds.token }
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/unpublish.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class Unpublish extends BaseCommand {
return []
}

const opts = this.npm.flatOptions
const opts = { ...this.npm.flatOptions, log }
const username = await getIdentity(this.npm, { ...opts }).catch(() => null)
if (!username) {
return []
Expand Down
3 changes: 2 additions & 1 deletion lib/commands/whoami.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const getIdentity = require('../utils/get-identity.js')
const log = require('../utils/log-shim.js')

const BaseCommand = require('../base-command.js')
class Whoami extends BaseCommand {
Expand All @@ -7,7 +8,7 @@ class Whoami extends BaseCommand {
static params = ['registry']

async exec (args) {
const username = await getIdentity(this.npm, this.npm.flatOptions)
const username = await getIdentity(this.npm, { ...this.npm.flatOptions, log })
this.npm.output(
this.npm.config.get('json') ? JSON.stringify(username) : username
)
Expand Down
5 changes: 3 additions & 2 deletions test/lib/commands/access.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,13 @@ t.test('access public on unscoped package', async t => {
})

t.test('access public on scoped package', async t => {
t.plan(2)
t.plan(3)
const name = '@scoped/npm-access-public-pkg'
const { npm } = await loadMockNpm(t, {
mocks: {
libnpmaccess: {
public: (pkg, { registry }) => {
public: (pkg, { registry, log }) => {
t.ok(log, 'should pass a logger')
t.equal(pkg, name, 'should use pkg name ref')
t.equal(
registry,
Expand Down
10 changes: 10 additions & 0 deletions test/lib/commands/deprecate.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@ const t = require('tap')

let getIdentityImpl = () => 'someperson'
let npmFetchBody = null
let npmFetchLog = null

const npmFetch = async (uri, opts) => {
npmFetchBody = opts.body
npmFetchLog = opts.log
}

npmFetch.json = async (uri, opts) => {
npmFetchLog = opts.log
return {
versions: {
'1.0.0': {},
Expand Down Expand Up @@ -82,7 +85,12 @@ t.test('invalid semver range', async t => {
})

t.test('undeprecate', async t => {
t.teardown(() => {
npmFetchBody = null
npmFetchLog = null
})
await deprecate.exec(['foo', ''])
t.ok(npmFetchLog, 'was passed a logger')
t.match(npmFetchBody, {
versions: {
'1.0.0': { deprecated: '' },
Expand All @@ -95,9 +103,11 @@ t.test('undeprecate', async t => {
t.test('deprecates given range', async t => {
t.teardown(() => {
npmFetchBody = null
npmFetchLog = null
})

await deprecate.exec(['foo@1.0.0', 'this version is deprecated'])
t.ok(npmFetchLog, 'was passed a logger')
t.match(npmFetchBody, {
versions: {
'1.0.0': {
Expand Down
3 changes: 2 additions & 1 deletion test/lib/commands/diff.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,10 @@ const diff = new Diff(npm)

t.test('no args', t => {
t.test('in a project dir', async t => {
t.plan(3)
t.plan(4)

libnpmdiff = async ([a, b], opts) => {
t.ok(opts.log, 'should be passed a logger')
t.equal(a, 'foo@latest', 'should have default spec comparison')
t.equal(b, `file:${fooPath}`, 'should compare to cwd')
t.match(opts, npm.flatOptions, 'should forward flat options')
Expand Down
14 changes: 13 additions & 1 deletion test/lib/commands/dist-tag.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,19 @@ const routeMap = {
// XXX overriding this does not appear to do anything, adding t.plan to things
// that use it fails the test
let npmRegistryFetchMock = (url, opts) => {
npmRegistryFetchLog = opts.log
if (url === '/-/package/foo/dist-tags') {
throw new Error('no package found')
}

return routeMap[url]
}

npmRegistryFetchMock.json = async (url, opts) => routeMap[url]
let npmRegistryFetchLog
npmRegistryFetchMock.json = async (url, opts) => {
npmRegistryFetchLog = opts.log
return routeMap[url]
}

const logger = (...msgs) => {
for (const msg of [...msgs]) {
Expand Down Expand Up @@ -81,13 +86,18 @@ const npm = mockNpm({
})
const distTag = new DistTag(npm)

t.afterEach(() => {
npmRegistryFetchLog = null
})

t.test('ls in current package', async t => {
npm.prefix = t.testdir({
'package.json': JSON.stringify({
name: '@scoped/pkg',
}),
})
await distTag.exec(['ls'])
t.ok(npmRegistryFetchLog, 'is passed a logger')
t.matchSnapshot(
result,
'should list available tags for current package'
Expand Down Expand Up @@ -289,6 +299,7 @@ t.test('add new tag', async t => {
})

npmRegistryFetchMock = async (url, opts) => {
t.ok(opts.log, 'is passed a logger')
t.equal(opts.method, 'PUT', 'should trigger request to add new tag')
t.equal(opts.body, '7.7.7', 'should point to expected version')
}
Expand Down Expand Up @@ -355,6 +366,7 @@ t.test('remove existing tag', async t => {
}
npm.prefix = t.testdir({})
await distTag.exec(['rm', '@scoped/another', 'c'])
t.ok(npmRegistryFetchLog, 'is passed a logger')
t.matchSnapshot(log, 'should log remove info')
t.matchSnapshot(result, 'should return success msg')
})
Expand Down
Loading