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

Suggestion: follow the user's npmrc config by default #689

Merged
merged 2 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ This plugin shows a warning message if a user is running an out of date CLI.

# How it works

This checks the version against the npm registry asynchronously in a forked process once every 60 days by default (see [Configuration](#configuration) for how to configure this). It then saves a version file to the cache directory that will enable the warning. The upside of this method is that it won't block a user while they're using your CLI—the downside is that it will only display _after_ running a command that fetches the new version.
This checks the version against the npm registry asynchronously in a forked process once every 7 days by default (see [Configuration](#configuration) for how to configure this). It then saves a version file to the cache directory that will enable the warning. The upside of this method is that it won't block a user while they're using your CLI—the downside is that it will only display _after_ running a command that fetches the new version.

# Installation

Expand All @@ -49,8 +49,8 @@ any of the following configuration properties:

- `timeoutInDays` - Duration between update checks. Defaults to 60.
- `message` - Customize update message.
- `registry` - URL of registry. Defaults to the public npm registry: `https://registry.npmjs.org`
- `authorization` - Authorization header value for registries that require auth.
- `registry` - URL of registry. Defaults to following your .npmrc configuration
- `authorization` - Authorization header value for registries that require auth. Defaults to following your .npmrc configuration
- `frequency` - The frequency that the new version warning should be shown.
- `frequencyUnit` - The unit of time that should be used to calculate the frequency (`days`, `hours`, `minutes`, `seconds`, `milliseconds`). Defaults to `minutes`.

Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"ansis": "^3.3.1",
"debug": "^4.3.5",
"http-call": "^5.2.2",
"lodash": "^4.17.21"
"lodash": "^4.17.21",
"registry-auth-token": "^5.0.2"
},
"devDependencies": {
"@commitlint/config-conventional": "^19",
Expand Down
6 changes: 3 additions & 3 deletions src/get-version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,16 @@ async function run([name, file, version, registry, authorization]: string[]) {
debug('file:', file)
debug('version:', version)
debug('registry:', registry)
debug('authorization:', authorization)

const url = [
registry.replace(/\/+$/, ''), // remove trailing slash
name.replace('/', '%2f'), // scoped packages need escaped separator
].join('/')
const headers = authorization ? {authorization} : {}
await mkdir(dirname(file), {recursive: true})
await writeFile(file, JSON.stringify({current: version, headers})) // touch file with current version to prevent multiple updates
await writeFile(file, JSON.stringify({current: version})) // touch file with current version to prevent multiple updates
const {body} = await HTTP.get<{'dist-tags': string[]}>(url, {headers, timeout: 5000})
await writeFile(file, JSON.stringify({...body['dist-tags'], authorization, current: version}))
await writeFile(file, JSON.stringify({...body['dist-tags'], current: version}))
// eslint-disable-next-line n/no-process-exit, unicorn/no-process-exit
process.exit(0)
}
Expand Down
21 changes: 17 additions & 4 deletions src/hooks/init/check-update.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
/* eslint-disable valid-jsdoc */

import {Hook, Interfaces} from '@oclif/core'
import {Ansis} from 'ansis'
import makeDebug from 'debug'
import {spawn} from 'node:child_process'
import {readFile, stat, writeFile} from 'node:fs/promises'
import {dirname, join, resolve} from 'node:path'
import {fileURLToPath} from 'node:url'
import getAuthToken from 'registry-auth-token';
import getRegistryUrl from 'registry-auth-token/registry-url.js';

const ansis = new Ansis()

Expand Down Expand Up @@ -130,15 +133,25 @@ const hook: Hook.Init = async function ({config}) {
const debug = makeDebug('update-check')
const versionFile = join(config.cacheDir, 'version')
const lastWarningFile = join(config.cacheDir, 'last-warning')
const scope = config.name.split('/')[0];

// Destructure package.json configuration with defaults
const {
authorization = '',
message = '<%= config.name %> update available from <%= chalk.greenBright(config.version) %> to <%= chalk.greenBright(latest) %>.',
registry = config.npmRegistry ?? 'https://registry.npmjs.org',
timeoutInDays = 60,
registry = config.npmRegistry ?? getRegistryUrl(scope), // Use custom registry or fallback to 1) registry set for the scope, 2) default registry in the npmrc, or 3) the default registry
timeoutInDays = 7,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why change the default timeoutInDays?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair question as it's not really relevant to this change in general, but this felt like a much more reasonable default for most tools. It's configurable so not a big deal but two months is a long time to wait (by default) to notify users of a new version.

} = config.pjson.oclif['warn-if-update-available'] ?? {}

// Get the authorization header next as we need the registry to be computed first
let {
authorization
} = config.pjson.oclif['warn-if-update-available'] ?? {}

if (!authorization) {
const authToken = getAuthToken(registry);
authorization = authToken ? `${authToken.type} ${authToken.token}` : '';
}

const refreshNeeded = async () => {
if (this.config.scopedEnvVarTrue('FORCE_VERSION_CACHE_UPDATE')) return true
if (this.config.scopedEnvVarTrue('SKIP_NEW_VERSION_CHECK')) return false
Expand Down Expand Up @@ -175,7 +188,7 @@ const hook: Hook.Init = async function ({config}) {
this.warn(
lodash.default.template(message)({
ansis,
// chalk and ansis have the same api. Keeping chalk for backwards compatibility.
// Chalk and ansis have the same api. Keeping chalk for backwards compatibility.
chalk: ansis,
config,
latest: newerVersion,
Expand Down