Skip to content

Commit

Permalink
fix(config): user-agent properly shows ci
Browse files Browse the repository at this point in the history
The way we were flattening user-agent back into itself meant that any
values that were dependent on other config items would never be seen,
since we have to re-flatten the item for each one it depends on.

We also weren't re-flattening the user-agent when setting workspaces or
workspace, which were things that affected the final result.

This does change the main config value of `user-agent` but not the
flattened one.  We are not using the main config value anywhere (which
is correct).
  • Loading branch information
wraithgar committed Sep 14, 2021
1 parent e4a5218 commit 54087b0
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 7 deletions.
12 changes: 11 additions & 1 deletion lib/utils/config/definitions.js
Expand Up @@ -2053,10 +2053,14 @@ define('user-agent', {
.replace(/\{workspaces\}/gi, inWorkspaces)
.replace(/\{ci\}/gi, ciName ? `ci/${ciName}` : '')
.trim()

// We can't clobber the original or else subsequent flattening will fail
// (i.e. when we change the underlying config values)
// obj[key] = flatOptions.userAgent

// user-agent is a unique kind of config item that gets set from a template
// and ends up translated. Because of this, the normal "should we set this
// to process.env also doesn't work
obj[key] = flatOptions.userAgent
process.env.npm_config_user_agent = flatOptions.userAgent
},
})
Expand Down Expand Up @@ -2140,6 +2144,9 @@ define('workspace', {
a workspace which does not yet exist, to create the folder and set it
up as a brand new workspace within the project.
`,
flatten: (key, obj, flatOptions) => {
definitions['user-agent'].flatten('user-agent', obj, flatOptions)
},
})

define('workspaces', {
Expand All @@ -2151,6 +2158,9 @@ define('workspaces', {
Enable running a command in the context of **all** the configured
workspaces.
`,
flatten: (key, obj, flatOptions) => {
definitions['user-agent'].flatten('user-agent', obj, flatOptions)
},
})

define('yes', {
Expand Down
4 changes: 2 additions & 2 deletions tap-snapshots/test/lib/config.js.test.cjs
Expand Up @@ -146,7 +146,7 @@ exports[`test/lib/config.js TAP config list --json > output matches snapshot 1`]
"unicode": false,
"update-notifier": true,
"usage": false,
"user-agent": "npm/{NPM-VERSION} node/{NODE-VERSION} {PLATFORM} {ARCH} workspaces/false",
"user-agent": "npm/{npm-version} node/{node-version} {platform} {arch} workspaces/{workspaces} {ci}",
"version": false,
"versions": false,
"viewer": "{VIEWER}",
Expand Down Expand Up @@ -296,7 +296,7 @@ umask = 0
unicode = false
update-notifier = true
usage = false
user-agent = "npm/{NPM-VERSION} node/{NODE-VERSION} {PLATFORM} {ARCH} workspaces/false"
user-agent = "npm/{npm-version} node/{node-version} {platform} {arch} workspaces/{workspaces} {ci}"
; userconfig = "{HOME}/.npmrc" ; overridden by cli
version = false
versions = false
Expand Down
34 changes: 30 additions & 4 deletions test/lib/utils/config/definitions.js
Expand Up @@ -747,15 +747,15 @@ t.test('user-agent', t => {
definitions['user-agent'].flatten('user-agent', obj, flat)
t.equal(flat.userAgent, expectNoCI)
t.equal(process.env.npm_config_user_agent, flat.userAgent, 'npm_user_config environment is set')
t.equal(obj['user-agent'], flat.userAgent, 'config user-agent template is translated')
t.not(obj['user-agent'], flat.userAgent, 'config user-agent template is not translated')

obj['ci-name'] = 'foo'
obj['user-agent'] = definitions['user-agent'].default
const expectCI = `${expectNoCI} ci/foo`
definitions['user-agent'].flatten('user-agent', obj, flat)
t.equal(flat.userAgent, expectCI)
t.equal(process.env.npm_config_user_agent, flat.userAgent, 'npm_user_config environment is set')
t.equal(obj['user-agent'], flat.userAgent, 'config user-agent template is translated')
t.not(obj['user-agent'], flat.userAgent, 'config user-agent template is not translated')

delete obj['ci-name']
obj.workspaces = true
Expand All @@ -764,15 +764,15 @@ t.test('user-agent', t => {
definitions['user-agent'].flatten('user-agent', obj, flat)
t.equal(flat.userAgent, expectWorkspaces)
t.equal(process.env.npm_config_user_agent, flat.userAgent, 'npm_user_config environment is set')
t.equal(obj['user-agent'], flat.userAgent, 'config user-agent template is translated')
t.not(obj['user-agent'], flat.userAgent, 'config user-agent template is not translated')

delete obj.workspaces
obj.workspace = ['foo']
obj['user-agent'] = definitions['user-agent'].default
definitions['user-agent'].flatten('user-agent', obj, flat)
t.equal(flat.userAgent, expectWorkspaces)
t.equal(process.env.npm_config_user_agent, flat.userAgent, 'npm_user_config environment is set')
t.equal(obj['user-agent'], flat.userAgent, 'config user-agent template is translated')
t.not(obj['user-agent'], flat.userAgent, 'config user-agent template is not translated')
t.end()
})

Expand Down Expand Up @@ -853,3 +853,29 @@ t.test('package-lock-only', t => {
t.strictSame(flat, { packageLock: false, packageLockOnly: false })
t.end()
})

t.test('workspaces', t => {
const obj = {
workspaces: true,
'user-agent': definitions['user-agent'].default,
'npm-version': '1.2.3',
'node-version': '9.8.7',
}
const flat = {}
definitions.workspaces.flatten('workspaces', obj, flat)
t.equal(flat.userAgent, 'npm/1.2.3 node/9.8.7 darwin x64 workspaces/true')
t.end()
})

t.test('workspace', t => {
const obj = {
workspace: ['workspace-a'],
'user-agent': definitions['user-agent'].default,
'npm-version': '1.2.3',
'node-version': '9.8.7',
}
const flat = {}
definitions.workspace.flatten('workspaces', obj, flat)
t.equal(flat.userAgent, 'npm/1.2.3 node/9.8.7 darwin x64 workspaces/true')
t.end()
})

0 comments on commit 54087b0

Please sign in to comment.